Ultimaker / Cura

3D printer / slicing GUI built on top of the Uranium framework
GNU Lesser General Public License v3.0
6.08k stars 2.06k forks source link

Dual extruder fan speed setting crossed-wired #9301

Open mah115 opened 3 years ago

mah115 commented 3 years ago

Application version Cura 4.8.0 Platform Windows 10

Printer WEEDO X40 (custom profile)

Reproduction steps

  1. Slice the file

Screenshot(s) Extruder Settings: image Cooling Settings: image

Actual results Generated gcode:

;LAYER:1 G1 F1200 E2.10241 G1 F600 Z1.35 G92 E0 T1 G92 E0 T1 S M105 M109 S200 M106 S25.5 P2 G1 F1200 E11.5 G0 F4800 X0.00 Y0.00 Z1.35 G1 F600 Z0.35 G0 F4800 X150.46 Y150.001 ... ;LAYER:2 G1 F1200 E73.34988 G1 F600 Z1.5 G92 E0 T0 G92 E0 T0 S M105 M109 S250 M104 T1 S0 M106 S127.5 P1

Expected results When the printer grabs extruder T1, the fan speed should have been 50% on fan 2 (M106 S127.5 P2). When the printer grabs extruder T0, the fan speed gcode should have been 10% on fan 1 (M106 S25.5 P1)

Project file CFFFP_coupon.zip

smartavionics commented 3 years ago

The issue is being triggered by the fact that only one extruder is used on each of layers 2 and 3. It was trying to reuse an extruder plan initialised for the other extruder's fan settings but it should have been creating a new extruder plan for the new extruder. The following patch fixes this but has not been tested with any other multi-extruder prints or configurations so YMMV...

diff --git a/src/LayerPlan.cpp b/src/LayerPlan.cpp
index f4406567..04184251 100644
--- a/src/LayerPlan.cpp
+++ b/src/LayerPlan.cpp
@@ -327,9 +327,8 @@ bool LayerPlan::setExtruder(const size_t extruder_nr)
             addTravel(end_pos); //  + extruder_offset cause it
         }
     }
-    if (extruder_plans.back().paths.empty() && extruder_plans.back().inserts.empty())
+    if (extruder_plans.back().paths.empty() && extruder_plans.back().inserts.empty() && extruder_plans.back().extruder_nr == extruder_nr)
     { // first extruder plan in a layer might be empty, cause it is made with the last extruder planned in the previous layer
-        extruder_plans.back().extruder_nr = extruder_nr;
     }
     else 
     {
smartavionics commented 3 years ago

Perhaps this is sufficient...

diff --git a/src/LayerPlan.cpp b/src/LayerPlan.cpp
index f4406567..85b241fe 100644
--- a/src/LayerPlan.cpp
+++ b/src/LayerPlan.cpp
@@ -327,11 +327,7 @@ bool LayerPlan::setExtruder(const size_t extruder_nr)
             addTravel(end_pos); //  + extruder_offset cause it
         }
     }
-    if (extruder_plans.back().paths.empty() && extruder_plans.back().inserts.empty())
-    { // first extruder plan in a layer might be empty, cause it is made with the last extruder planned in the previous layer
-        extruder_plans.back().extruder_nr = extruder_nr;
-    }
-    else 
+    if (extruder_plans.back().extruder_nr != extruder_nr)
     {
         extruder_plans.emplace_back(extruder_nr, layer_nr, is_initial_layer, is_raft_layer, layer_thickness, fan_speed_layer_time_settings_per_extruder[extruder_nr], storage.retraction_config_per_extruder[extruder_nr]);
         assert(extruder_plans.size() <= Application::getInstance().current_slice->scene.extruders.size() && "Never use the same extruder twice on one layer!");
smartavionics commented 3 years ago

In fact, the test isn't required at all because it's already been tested at the top of the function so it gets even simpler...

diff --git a/src/LayerPlan.cpp b/src/LayerPlan.cpp
index f4406567..e06c5cf7 100644
--- a/src/LayerPlan.cpp
+++ b/src/LayerPlan.cpp
@@ -327,15 +327,8 @@ bool LayerPlan::setExtruder(const size_t extruder_nr)
             addTravel(end_pos); //  + extruder_offset cause it
         }
     }
-    if (extruder_plans.back().paths.empty() && extruder_plans.back().inserts.empty())
-    { // first extruder plan in a layer might be empty, cause it is made with the last extruder planned in the previous layer
-        extruder_plans.back().extruder_nr = extruder_nr;
-    }
-    else 
-    {
-        extruder_plans.emplace_back(extruder_nr, layer_nr, is_initial_layer, is_raft_layer, layer_thickness, fan_speed_layer_time_settings_per_extruder[extruder_nr], storage.retraction_config_per_extruder[extruder_nr]);
-        assert(extruder_plans.size() <= Application::getInstance().current_slice->scene.extruders.size() && "Never use the same extruder twice on one layer!");
-    }
+    extruder_plans.emplace_back(extruder_nr, layer_nr, is_initial_layer, is_raft_layer, layer_thickness, fan_speed_layer_time_settings_per_extruder[extruder_nr], storage.retraction_config_per_extruder[extruder_nr]);
+    assert(extruder_plans.size() <= Application::getInstance().current_slice->scene.extruders.size() && "Never use the same extruder twice on one layer!");
     last_planned_extruder = &Application::getInstance().current_slice->scene.extruders[extruder_nr];

     { // handle starting pos of the new extruder
mah115 commented 3 years ago

Should I reopen this ticket under CuraEngine repo? Or maybe you should do it and add a pull request? (I'm not a programmer, I'm still trying to figure out how git works...)

smartavionics commented 3 years ago

No, leave it as is, the Cura devs will see it here OK.

fvrmr commented 3 years ago

Hi @mah115 thank you for your bug report and thank you @smartavionics for looking into it and creating a patch. I will discuss this with the team. Keep you posted!

fvrmr commented 3 years ago

Hi @mah115 I have discussed it with the team, and I was wondering if your project file is the same as you used for the gcode that is in your zip. Because when I generated gcode from your project file the M106 isn't in it at all. Tried it on our master and 4.8. CFFFP_coupon-v1.gcode.zip

We need to reproduce this first before we can add the patch of @smartavionics.

mah115 commented 3 years ago

I'm not sure why it's not working for you, I downloaded the original zip file and tried it again and the results are the same. Here's exactly what I'm doing: https://youtu.be/eG72BTgG0gw

My computer specs: image

mah115 commented 3 years ago

Hi @mah115 I have discussed it with the team, and I was wondering if your project file is the same as you used for the gcode that is in your zip. Because when I generated gcode from your project file the M106 isn't in it at all. Tried it on our master and 4.8. CFFFP_coupon-v1.gcode.zip

We need to reproduce this first before we can add the patch of @smartavionics.

Wouldn't it be even worse if the part cooling is selected but M106 isn't in the gcode? Either way it's a bug...

fvrmr commented 3 years ago

I'm not sure why it's not working for you, I downloaded the original zip file and tried it again and the results are the same. Here's exactly what I'm doing: https://youtu.be/eG72BTgG0gw

I did exactly the same but still M106 isn't showing. I tried it also with an other object with the same settings and printer and then it is showing one extruder speed settings.

M106 S25.5 P1
;TYPE:WALL-INNER
;MESH:cube.stl(1)
G1 F1500 E32.37657
G1 F900 X145.75 Y165.25 E32.64161
G1 X145.75 Y156.75 E32.90665
G1 X154.25 Y156.75 E33.1717
G1 X154.25 Y165.25 E33.43674
G0 F5040 X154.75 Y165.75

Wouldn't it be even worse if the part cooling is selected but M106 isn't in the gcode? Either way it's a bug...

Yes but if it's a bug, but we need to reproduce first to solve it.

Does it also happen with other files? If so could you share one of those.

smartavionics commented 3 years ago

You realise that the bug will also cause the wrong retraction settings to be used as well as the wrong fan settings?

mah115 commented 3 years ago

Wouldn't it be even worse if the part cooling is selected but M106 isn't in the gcode? Either way it's a bug...

Yes but if it's a bug, but we need to reproduce first to solve it.

What I was trying to say is that if the cooling fan option is selected on both extruders, and M106 doesn't even show up in the gcode, then you've reproduced a certain bug--maybe not the same bug, but something worth investigating?

I've made a new file: coupon2.zip What I did was put in the coupon.STL object and raised it off the platform by 0.4mm, and enable supports. So the first 2 layers should be printed with extruder 2 and the rest printed with extruder 1.

Not sure if this will help debug the issue of the gcode generation being different on our systems, but here's a zip of my CURA installation folder: https://www.dropbox.com/s/0ntwbcdj6fh5400/Ultimaker%20Cura%204.8.0.zip?dl=0 and the files from /User/AppData/Roaming: https://www.dropbox.com/s/r8klxmqcmayu2mz/cura.zip?dl=0 I'm wondering, are you also using Windows?

fvrmr commented 3 years ago

What I was trying to say is that if the cooling fan option is selected on both extruders, and M106 doesn't even show up in the gcode, then you've reproduced a certain bug--maybe not the same bug, but something worth investigating?

That's true.

I'm wondering, are you also using Windows?

Yes I am also using Windows.

Thanks for your new file, I can reproduce it with this. Will discuss it again with the team. Keep you posted.

rburema commented 3 years ago

Working on some PR's right now, including the linked one. Figured out the discrepancy between what we're seeing and what was reported: It's the 'auto drop to buildplate' thing again.

Switch that off and the files become at least similar enough to reproduce the original problem.

Thought I'd just leave that here in case I don't get any further with the PR(s).