Custom G-code: wrong extruder IDs injected

[platsch]

Version

1.3.0 dev

Behavior

The placeholder variable for extruder IDs e.g. for tool change G-code are directly evaluated to the config values, starting with 1. This behavior makes it impossible to use placeholders as gcode variables, since the firmware counts extruders starting with 0.
[previous_extruder] and [next_extruder] are correctly replaced, i.e. starting with 0.

Example:
;perimeter_extruder: [perimeter_extruder] -> 1
;previous_extruder: [previous_extruder] -> 0
but both are actually the same extruder.

The reason is, that GCode::set_extruder explicitly injects the correct "special" variables but directly imports all settings via PlaceholderParser::process.

Reproduction files

3574.zip

[lordofhyphens]

@platsch I've edited your post to include a reproduction test case. I've set the perimeter extruder to 2 and left infill at 1 (in config) and added output gcode for tool change that writes.

In the provided output g code, confirm by observing line 96-100 and 1226-1229.

[lordofhyphens]

I can see one of two places to fix this, either in the UI (by changing the initial ID to 0 from 1) or libslic3r.

I think fixing it in the config/UI side is the appropriate option as it has less opportunity for special-casing, but reading older configs will cause the program to do things people don't expect.

[platsch]

Actually, I considered providing a PR to fix this and the reason I didn't was exactly this question.

I have the impression that fixing the UI is the cleaner solution since the extruder numbering was always inconsistent with the firmware. On the other hand: what was the original reason to design the GUI as it is? Are we neglecting an important point?

Breaking basically all existing configs would probably be a major problem...

[bubnikv]

Another solution, IMHO better one, would be to finally implement an
expression parser. A simple one, supporting braces, +-* operators would be
sufficient.

It is really just a convention, whether the extruders are numbered from 0
or 1. Programmers like the zero, but non-programmers 1. IMHO it would be
better to keep it as it is and only handle the conversion at the macro
expansion.

On Fri, Nov 18, 2016 at 9:19 AM, platsch [email protected] wrote:

Actually, I considered providing a PR to fix this and the reason I didn't
was exactly this question.

I have the impression that fixing the UI is the cleaner solution since the
extruder numbering was always inconsistent with the firmware. On the other
hand: what was the original reason to design the GUI as it is? Are we
neglecting an important point?

Breaking basically all existing configs would probably be a major
problem...

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#3574 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AFj5I3SKCj5Rp1x0lbCqyBAObBlAJl81ks5q_V-jgaJpZM4KxYus
.

[alranel]

Oh. Right. Damn, I should have used zero-numbering everywhere. It makes no sense to use different numbers than what machine uses.

I'm afraid now it's too late to switch how they are stored in config files in backwards compatible way. Maybe we should just subtract 1 whenever we use them as placeholders.

[alranel]

I think 0 in the GUI has a special meaning now, though (not sure)

[platsch]

@bubnikv an expression parser would technically solve this problem, but it is not very intuitive to have some variables counting from 0 and some from 1 in the same evaluation context.
I think it is too late to change the GUI numbering. My preference would be 0-numbering for all variables in the context of gcode, because that's more consistent to other (static) gcode commands, and a good documentation (tooltip).

I suggest we let the PlaceholderParser subtract 1 for all extruder-related placeholders. Sadly, this requires a number of parser exceptions, but I just see no better solution. Does this affect anything aside from the 5 extruder variables?

[bubnikv]

I will use the "0th" extruder for the support and support interface extruders to indicate "does not matter, use the last one" to minimize the number of extruder exchanges. Therefore it makes sense to reserve "0" as a "default" value as it is done everywhere else in Slic3r.

[bubnikv]

@platsch I think it is too late to change the GUI numbering. My preference would be 0-numbering for all variables in the context of gcode, because that's more consistent to other (static) gcode commands, and a good documentation (tooltip).
I suggest we let the PlaceholderParser subtract 1 for all extruder-related placeholders. Sadly, this requires a number of parser exceptions, but I just see no better solution. Does this affect anything aside from the 5 extruder variables?

Maybe we just need to change
GCode::set_extruder(unsigned int extruder_id)
and decrease the extruder id at the two following lines:
pp.set("previous_extruder", this->writer.extruder()->id);
pp.set("next_extruder", extruder_id);

No need to change the expression parser.

Or just add another two sets of variables:
pp.set("previous_extruder_minus_1", this->writer.extruder()->id-1);
pp.set("next_extruder_minus_1", extruder_id-1);
or whatever suffix will be more explanatory:
previous_extruder_minus_1
previous_extruder_m1
previous_extruder_zero_based
previous_extruder_0based
previous_extruder_gcode
whatever

The 2nd solution would be backward compatible.