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

Tool 0 not selected at start of GCode #1819

Closed
tcm0116 opened this issue May 12, 2017 · 32 comments · Fixed by Ultimaker/CuraEngine#640
Closed

Tool 0 not selected at start of GCode #1819

tcm0116 opened this issue May 12, 2017 · 32 comments · Fixed by Ultimaker/CuraEngine#640
Labels
Type: Bug The code does not produce the intended behavior.

Comments

@tcm0116
Copy link

tcm0116 commented May 12, 2017

With Cura 2.5 in a dual-extruder setup, if a part is sliced using the first extruder, a T0 command is never placed in the corresponding GCode. This can be an issue if the print is started with the printer having T1 as the active extruder.

@fieldOfView
Copy link
Collaborator

Can't you put a T0 command in the start G-code?

@tcm0116
Copy link
Author

tcm0116 commented May 12, 2017

That's what I've done, and I think it'll work. It just seems to me that Cura should always be sure to select the correct tool in a multi-extruder setup.

@maukcc
Copy link
Contributor

maukcc commented May 12, 2017

Would it be possible to put a Tx command ,for the 1st extruder that is going to be used, at the very beginning of the file? So even before the heating codes that Cura generates. This would solve a couple of issues, but would require that the heating codes generated by Cura must all have Tx in them.

@maukcc
Copy link
Contributor

maukcc commented May 15, 2017

This can be an issue if the print is started with the printer having T1 as the active extruder

Also, putting the T0 in your start gcode will give an issue, when you start your print with T1

@Ghostkeeper
Copy link
Collaborator

We can only do this if we're sure that no printer at all has any problem with the T0 command while already being at T0. I mean, it doesn't switch unnecessarily, or heat the nozzle, or whatever. If there's any printer that does this, it's better for all printers that don't guarantee T0 at the start to have T0 in the start g-code.

@maukcc
Copy link
Contributor

maukcc commented May 16, 2017

Putting heating codes in the start gcode, on a multi extruder setup, does not work as Cura does not know what extruder is going to be used 1st when generating the start gcode, and therefore puts in a default temp(usually 210 degrees).

Therefore you need Cura to put in the heatercodes. this happens before the start gcode.
For T0, Cura generates M104 S185 as it assumes T0 is the active extruder and will be the 1st extruder used(both faulty assumptions), which is an incomplete code. It should be M104 T0 S185(regardless of what firmware is used)
So now the start looks like(also when T1 is the active extruder)

M104 S185
M104 T1 S185
M104 T2 S185
M104 T3 S185

Which results with an unheated T0 if T1 is the active extruder.

When heating is done correct, all heaters are heated, which is correct and wanted behavior as we want to clean all nozzles before use. But it would be silly if only T1 is going to be used and all other heaters stay hot. This will even damage hardware.
So now we need to cool down the unused heaters. We do this in start gcode with

M104 S21 T1
M104 S21 T2
M104 S21 T3

This works if you always start with a skirt/brim or something else made with T0.
That is also silly if you just want to use T1 for your project as T0 then stays heated
So for general purpose you need to cool all heaters down

M104 S21 T0
M104 S21 T1
M104 S21 T2
M104 S21 T3

But now the extruder that is going to be used is also cooled down.

If you use T1 (or any other extruder except T0) as 1st extruder, Cura sees this as a toolchange and starts with a heatup code for that extruder.
Which is super, exactly what we need. We just need Cura to do this for T0 as well.
One flaw here is that in a "start" toolchange you also get the "cooldown" for the previous extruder. Which actually heats up the unused extruder to material_standby_temperature at start.
Cooling the previous extruder to material_standby_temperature should be done at "end" toolchange of that previous extruder.

So Cura should always do a toolchange between start gcode and actual print code. Then temps will always be correct.

So 3 thing need to be done to have a correct working general purpose start:
1 correct heating code for T0
2 always a toolchange at the start of the regular print (do not assume the print will start with T0)
3 put the material_standby_temperature code at the "end" toolchange

Ideas??

@BagelOrb
Copy link
Contributor

I think all of these problems are already solved by putting T0 in your start gcode.
Cura already assumes you start on T0 if I'm correct...

@BagelOrb BagelOrb added the Type: Bug The code does not produce the intended behavior. label Jul 21, 2017
@Ghostkeeper
Copy link
Collaborator

Except that this introduces two unnecessary tool changes, and heats up the unused extruder from 0 to standby temperature at the start.

The use case for this is that some people use multiple-extrusion as redundancy or just easy material switching. When one nozzle breaks down, you can still continue printing if you just use the other one. You won't ever need to switch to the broken nozzle if you just single-extrusion print on the other nozzle.

@Ghostkeeper
Copy link
Collaborator

Our project manager just removed this from our planning because it's older than 12 weeks.

@maukcc
Copy link
Contributor

maukcc commented Oct 30, 2017

is it as simple to fix as Ultimaker/CuraEngine#633 says?

@tcm0116
Copy link
Author

tcm0116 commented Oct 30, 2017

Our project manager just removed this from our planning because it's older than 12 weeks.

Is that just a nice way of saying that you're not going to fix it because you haven't fixed it yet?

This is still an issue. Yes, I can put T0 in my start gcode, but what if I actually want to use T1 because it already has the filament loaded that I want? Now I have to go change my start gcode. Not very user friendly.

@fieldOfView
Copy link
Collaborator

How about including this in your start gcode:
T{adhesion_extruder_nr}, or T{top_bottom_extruder_nr} if you don't use skirt, brim or raft?

@tcm0116
Copy link
Author

tcm0116 commented Oct 30, 2017 via email

@fieldOfView
Copy link
Collaborator

fieldOfView commented Oct 30, 2017

Or, you can ignore people who try to be helpful and think of solutions to fix your problem.

@tcm0116
Copy link
Author

tcm0116 commented Oct 30, 2017 via email

@fieldOfView
Copy link
Collaborator

fieldOfView commented Oct 30, 2017

And where exactly did you point out a list of caveats?

I do work on the frontend. I can help you find a solution that works through the frontend. I was trying to find a solution that does not require you to wait at least 2 months for a new release, if it gets attention at all. Chances of this getting fixed in the backend look slim, because the issue does not affect Ultimaker printers.

@tcm0116
Copy link
Author

tcm0116 commented Oct 30, 2017

if you don't use skirt, brim or raft?

Isn't that a list of caveats?

I was trying to find a solution that does not require you to wait at least 2 months for a new release, if it gets attention at all.

And I appreciate that, I really do. Keep in mind that this issue has been open for over 5 months already, and it's just frustrating when you get the following:

Our project manager just removed this from our planning because it's older than 12 weeks.

Why is age the sole reason for dropping an issue? If anything, the older issues should get higher priority.

@fieldOfView
Copy link
Collaborator

One exception is not a list of caveats. I was suggesting something above, and if that worked it could be turned into a more full-fledged solution.

Note that I have no affiliation with Ultimaker (anymore), so I am not involved in what issues get dropped.

@fieldOfView
Copy link
Collaborator

The code in the backend where it probably should happen is here:
https://github.com/Ultimaker/CuraEngine/blob/master/src/FffGcodeWriter.cpp#L403
Note that for "Reprap" flavor, there is already a T0 included before the temperatures.

If I understand this issue correctly, just having the same T0 for "Marlin" flavor would only partially solve the problem, because it would still heat up hotend 0 even if you only print with hotend 1.

A proper solution would be for this code (https://github.com/Ultimaker/CuraEngine/blob/master/src/FffGcodeWriter.cpp#L410) to only heat the used extruders (see extruder_is_used) to their standby temperatures, and only wait for the layer_0 print temperature for the extruder start_extruder_nr.

@BagelOrb
Copy link
Contributor

In some printers switching extruder is a physical action, which depends on the previous extruder. It is then needed to always have a previous extruder defined, because changing extruder depends on the previous extruder. That means that a starting extruder must be assumed. When the printer starts it must have extruder zero selected.

According to this reasoning you may want to file a bug report at the firmware rather than at Cura.

Solving this in Cura is harder than you might think and I don't think ultimaker is going to spend time in making third party machines work with Cura.

Pull requests are always welcome of course!

@fieldOfView
Copy link
Collaborator

fieldOfView commented Oct 30, 2017

I have not even tried compiling this, but its an implementation of my above assumptions:
Ultimaker/CuraEngine@master...fieldOfView:fix_start_extruder_heatup

@smartavionics, you are much more versed in CuraEngine than I am, could you have a look if this would work?

@smartavionics
Copy link
Contributor

Hi @fieldOfView, I tried compiling your code and there's a { } mismatch. When I inserted the required { where I thought it would go it still doesn't compile.

The code currently looks like this:

        if (getSettingBoolean("material_print_temp_prepend"))
        {
            for (int extruder_nr = 0; extruder_nr < storage.getSettingAsCount("machine_extruder_count"); extruder_nr++)
            {
                if (extruder_is_used[extruder_nr])
                {
                    ExtruderTrain& train = *storage.meshgroup->getExtruderTrain(extruder_nr);
                    double extruder_temp;
                    if (extruder_nr == start_extruder_nr)
                    {
                        double print_temp_0 = train.getSettingInDegreeCelsius("material_print_temperature_layer_0");
                        extruder_temp = (print_temp_0 != 0)? print_temp_0 : train.getSettingInDegreeCelsius("material_print_temperature");
                    }
                    else
                    {
                        extruder_temp = train.getSettingInDegreeCelsius("material_standby_temperature");
                    }
                    gcode.writeTemperatureCommand(extruder_nr, extruder_temp);
                }
            }
            if (getSettingBoolean("material_print_temp_wait"))
            {
                if (extruder_is_used[extruder_nr])
                {
                    ExtruderTrain& train = *storage.meshgroup->getExtruderTrain(extruder_nr);
                    double extruder_temp;
                    if (extruder_nr == start_extruder_nr)
                    {
                        double print_temp_0 = train.getSettingInDegreeCelsius("material_print_temperature_layer_0");
                        extruder_temp = (print_temp_0 != 0)? print_temp_0 : train.getSettingInDegreeCelsius("material_print_temperature");
                    }
                    else
                    {
                        extruder_temp = train.getSettingInDegreeCelsius("material_standby_temperature");
                    }
                    gcode.writeTemperatureCommand(extruder_nr, extruder_temp, true);
                }
            }
        }

And the compilation fails like this:

/home/markb/3dp/CuraEngine.git/src/FffGcodeWriter.cpp: In member function ‘void cura::FffGcodeWriter::processStartingCode(const cura::SliceDataStorage&, unsigned int)’:
/home/markb/3dp/CuraEngine.git/src/FffGcodeWriter.cpp:427:21: error: ‘extruder_is_used’ was not declared in this scope
                 if (extruder_is_used[extruder_nr])
                     ^
/home/markb/3dp/CuraEngine.git/src/FffGcodeWriter.cpp:431:37: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
                     if (extruder_nr == start_extruder_nr)
                                     ^
/home/markb/3dp/CuraEngine.git/src/FffGcodeWriter.cpp:445:21: error: ‘extruder_is_used’ was not declared in this scope
                 if (extruder_is_used[extruder_nr])
                     ^
/home/markb/3dp/CuraEngine.git/src/FffGcodeWriter.cpp:445:38: error: ‘extruder_nr’ was not declared in this scope
                 if (extruder_is_used[extruder_nr])

So I assume that extruder_is_used needs to be declared somewhere, in the class?

Also you are referencing extruder_nr after it goes out of scope and I don't know what your intentions is there, either.

I am willing to help with this but I know nothing about multiple extruders so all I can do is take orders!

@fieldOfView
Copy link
Collaborator

Sorry, like I said I had not even compiled the code. I was asking more about the logic. I'll have a go at compiling and testing the code before asking for more feedback ;-)

@smartavionics
Copy link
Contributor

Not a problem, here's how I imagine the main part of that code should look (it compiles OK):

    if (gcode.getFlavor() != EGCodeFlavor::ULTIGCODE && gcode.getFlavor() != EGCodeFlavor::GRIFFIN)
    {
        std::ostringstream tmp;
        tmp << "T" << start_extruder_nr;
        gcode.writeLine(tmp.str().c_str());

        if (getSettingBoolean("material_bed_temp_prepend"))
        {
            if (getSettingBoolean("machine_heated_bed"))
            {
                double bed_temp = getSettingInDegreeCelsius("material_bed_temperature_layer_0");
                if (bed_temp != 0)
                {
                    gcode.writeBedTemperatureCommand(bed_temp, getSettingBoolean("material_bed_temp_wait"));
                }
            }
        }

        const unsigned num_extruders = storage.getSettingAsCount("machine_extruder_count");

        if (getSettingBoolean("material_print_temp_prepend"))
        {
            for (unsigned extruder_nr = 0; extruder_nr < num_extruders; extruder_nr++)
            {
                if (extruder_is_used[extruder_nr])
                {
                    ExtruderTrain& train = *storage.meshgroup->getExtruderTrain(extruder_nr);
                    double extruder_temp;
                    if (extruder_nr == start_extruder_nr)
                    {
                        double print_temp_0 = train.getSettingInDegreeCelsius("material_print_temperature_layer_0");
                        extruder_temp = (print_temp_0 != 0)? print_temp_0 : train.getSettingInDegreeCelsius("material_print_temperature");
                    }
                    else
                    {
                        extruder_temp = train.getSettingInDegreeCelsius("material_standby_temperature");
                    }
                    gcode.writeTemperatureCommand(extruder_nr, extruder_temp);
                }
            }
        }

        if (getSettingBoolean("material_print_temp_wait"))
        {
            for (unsigned extruder_nr = 0; extruder_nr < num_extruders; extruder_nr++)
            {
                if (extruder_is_used[extruder_nr])
                {
                    ExtruderTrain& train = *storage.meshgroup->getExtruderTrain(extruder_nr);
                    double extruder_temp;
                    if (extruder_nr == start_extruder_nr)
                    {
                        double print_temp_0 = train.getSettingInDegreeCelsius("material_print_temperature_layer_0");
                        extruder_temp = (print_temp_0 != 0)? print_temp_0 : train.getSettingInDegreeCelsius("material_print_temperature");
                    }
                    else
                    {
                        extruder_temp = train.getSettingInDegreeCelsius("material_standby_temperature");
                    }
                    gcode.writeTemperatureCommand(extruder_nr, extruder_temp, true);
                }
            }
        }
    }

At the top of the function, it now looks like this:

    std::vector<bool> extruder_is_used = storage.getExtrudersUsed();
    if (!CommandSocket::isInstantiated())
    {
        std::string prefix = gcode.getFileHeader(extruder_is_used);
        gcode.writeCode(prefix.c_str());
    }

@d235j
Copy link

d235j commented Oct 30, 2017

@BagelOrb

In some printers switching extruder is a physical action, which depends on the previous extruder. It is then needed to always have a previous extruder defined, because changing extruder depends on the previous extruder. That means that a starting extruder must be assumed. When the printer starts it must have extruder zero selected.
According to this reasoning you may want to file a bug report at the firmware rather than at Cura.

I'm not sure I fully understand this — if a print is completed using extruder 1, the printer may still be on extruder 1 as the active extruder has not been changed (and there is no reason for the printer to reset the extruder at the end of the print).

Can you cite a printer that has the behaviour you describe?

I believe the correct solution would be something in between the suggestion in #633 and the PR cited by @fieldOfView above:

  • Tool number should be output by startExtruder() no matter what the current extruder is (or alternatively, the current_extruder should be initialized to -1 so that on the very first startExtruder to extruder 0, the tool number is emitted) — this could lead to unwanted side-effects depending on how/when current_extruder is used, but should be fairly straightforward to debug — having currentExtruder assumed to be 0 even when the only extruder in use is 1 would cause problems anyway
  • Extruder heating commands should always include tool number
  • Extruder heating commands should not be output for unused extruders. However, for multiple extrusion prints, both extruders should be heated at the beginning of the print (or maybe there should be an option for this). This is related to Bug: second extruder train will be heat up also if only the first is required for print (prev. feature request "non"-Material for disabling extruder train on dual setup) #1128.

@BagelOrb
Copy link
Contributor

The Ultimaker 3 has this. I don't know much about other dual extruder machines, but there might also be other printers for which changing nozzles depends on the previous nozzle.

The Ultimaker 3 takes care of the current nozzle being T0 at the start.
However, I think it also simply puts T0 at the start of the gcode.

@d235j
Copy link

d235j commented Oct 30, 2017

@BagelOrb I figure this it the case on the UM3 because it has some mechanism to shift the active nozzle down.
I would assume the Magnetic Tool Changer for UM2 to depend on the previous nozzle in a similar way.

However, in both of these cases, setting the nozzle to the same as current should be a no-op to the printer — which means setting the nozzle to the start nozzle at the beginning of the print (as described in my above comment) would make sense. If the already-set nozzle is reselected, the printer would do nothing — and if the other nozzle was selected, the printer would switch it.

Basically what I'm saying is that assuming that the printer firmware resets the current tool/extruder to T0 before/after each print is a bad idea and that it is better to treat it as an uninitialized variable and explicitly set it.

@BagelOrb
Copy link
Contributor

I think that already happens for the UM3 - just to be sure.

For some machines that might not work. Some machines might interpret a T0 as "Go and heat T0 up". I'm not sure.

Either way just make a PR and it might very well be merged.

@d235j
Copy link

d235j commented Oct 30, 2017

@BagelOrb sounds good. I mainly don't want to break printers which behave in ways I'm not familiar with and I don't have to test (therefore my question about tool changes being dependent).

Rather than always putting T0 at the beginning, I'd have it only be generated with setExtruder/switchExtruder, if the extruder is previously uninitialized — which should avoid the "Go and heat T0 up" problem.

@tcm0116
Copy link
Author

tcm0116 commented Oct 31, 2017 via email

@fieldOfView
Copy link
Collaborator

I think that already happens for the UM3 - just to be sure.
Note that for the UM3, T0 is already inserted.

I have created a PR out of my code from above, after testing the code some more. Ultimaker/CuraEngine#640

@stuartpb
Copy link

stuartpb commented Jan 2, 2018

Another bug around how Cura assumes the extruder starts as T0: Ultimaker/CuraEngine#676

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug The code does not produce the intended behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants