Benign comment can cause GPX to generate a "change tool" command

[dcnewman]

When using the left extruder (tool 1), benign comments in the gcode cause GPX to generate a "change tool command" switching unexpectedly to the right extruder (tool 0).

Here's simplified gcode exhibiting this behavior. The Z axis has 400 steps/mm and so 324 steps = 0.81 mm and 444 steps is 1.11mm.

G90
M83
M73 P0; enable build progress
G21; set units to mm
G90; set positioning to absolute
G92 X0 Y0 Z0 A0 B0
M140 S100 T0
M104 S0 T0
M104 S210 T1
M6
G1 E-1 F1800
; layer 1, Z = 0.21
M135 T1
; tool H0.3 W0.48
G1 X14.72 Y-11.8 F3600
G1 Z0.21 F600
G1 E-1 F1800
; layer 2, Z = 0.51
G1 X-9.28 Y9.28 F3600
G1 Z0.51 F600
G1 E1 F1800
G1 E-1 F1800
; layer 3, Z = 0.81
M140 S74 T0
G1 X-9.28 Y9.28 F3600
G1 Z0.81 F600
G1 E1 F1800
G1 X-9.28 Y-9.28 E1.0474 F2700
G1 X8.896 Y8.465 E0.0244
G1 E-1 F1800
; layer 4, Z = 1.11
G1 X-9.28 Y9.28 E0 F3600
G1 Z1.11 E0 F600
G1 E1 F1800
M73 P100 ; end build progress

And here's the X3G dump. Note the switch to tool 0 on command 28. That's right at the end of "layer 3" for 0.81 mm and the start of layer 4 for 1.11mm. If the comment flagging the start of layer 4 is removed, the spurious tool change command goes away.

% s3g-decompiler.py cube2.x3g | more
Command number [byte offset]: Command name: options
1 [0]: Start build: steps 0, name "cube2"
2 [11]: Set build percentage: 0%, ignore 0
3 [14]: Switch tool: change to tool 0
4 [16]: Set machine position extended: (0,0,0,0,0)
5 [37]: Tool 0: Set build platform target temperature: 100
6 [43]: Set build percentage: 1%, ignore 0
7 [46]: Tool 0: Set target temperature: 0
8 [52]: Set build percentage: 80%, ignore 0
9 [55]: Tool 1: Set target temperature: 210
10 [61]: Set build percentage: 99%, ignore 0
11 [64]: Switch tool: change to tool 1
12 [66]: Wait on platform: tool 0, 100 ms between polls, 65535 s timeout
13 [72]: Wait on tool: wait for tool 1, 100 ms between polls, 65535 s timeout
14 [78]: Move to: (0,0,0,0,96), dda_rate 2560, relative mask 18, distance 1.000000, feedrateX64 1706
15 [110]: Move to: (1386,-1111,0,0,0), dda_rate 4407, relative mask 18, distance 18.865799, feedrateX64 3840
16 [142]: Move to: (1386,-1111,84,0,0), dda_rate 4000, relative mask 18, distance 0.210000, feedrateX64 640
17 [174]: Move to: (1386,-1111,84,0,97), dda_rate 2560, relative mask 18, distance 1.000000, feedrateX64 1706
18 [206]: Move to: (-874,874,84,0,0), dda_rate 4243, relative mask 18, distance 31.943174, feedrateX64 3840
19 [238]: Move to: (-874,874,204,0,0), dda_rate 3999, relative mask 18, distance 0.300000, feedrateX64 640
20 [270]: Move to: (-874,874,204,0,-97), dda_rate 2560, relative mask 18, distance 1.000000, feedrateX64 1706
21 [302]: Move to: (-874,874,204,0,97), dda_rate 2560, relative mask 18, distance 1.000000, feedrateX64 1706
22 [334]: Tool 0: Set build platform target temperature: 74
23 [340]: Move to: (-874,874,324,0,0), dda_rate 3999, relative mask 18, distance 0.300000, feedrateX64 640
24 [372]: Move to: (-874,874,324,0,-97), dda_rate 2560, relative mask 18, distance 1.000000, feedrateX64 1706
25 [404]: Move to: (-874,-874,324,0,-101), dda_rate 4235, relative mask 18, distance 18.559999, feedrateX64 2880
26 [436]: Move to: (837,797,324,0,-2), dda_rate 3031, relative mask 18, distance 25.401812, feedrateX64 2880
27 [468]: Move to: (837,797,324,0,96), dda_rate 2560, relative mask 18, distance 1.000000, feedrateX64 1706
28 [500]: Switch tool: change to tool 0
29 [502]: Move to: (-874,874,324,0,0), dda_rate 5642, relative mask 18, distance 18.194263, feedrateX64 3840
30 [534]: Move to: (-874,874,444,0,0), dda_rate 3999, relative mask 18, distance 0.300000, feedrateX64 640
31 [566]: Move to: (-874,874,444,-96,0), dda_rate 2560, relative mask 18, distance 1.000000, feedrateX64 1706
32 [598]: Set build percentage: 100%, ignore 0
33 [601]: End build: flags 0x0
34 [603]: Enable/disable axes: axes bitmask 1F
EOF

The offending code in GPX appears to be in the main processing loop around line 4595 of gpx.c,

else {
    // X,Y,Z,A,B,E,F
    if(gpx->command.flag & (AXES_BIT_MASK | F_IS_SET)) {
        CALL( calculate_target_position(gpx) );
        CALL( queue_ext_point(gpx, 0.0) );
        update_current_position(gpx);
        command_emitted++;
    }
    // Tn
    else if(!gpx->flag.dittoPrinting && gpx->target.extruder != gpx->current.extruder) {
        int timeout = gpx->command.flag & P_IS_SET ? (int)gpx->command.p : MAX_TIMEOUT;
        CALL( do_tool_change(gpx, timeout) );
        command_emitted++;
    }
}

That code triggers when a comment is handled. I assume it's meant for structured comments of significance to GPX. The comment here creates a case of gpx->target.extruder == 0 since there is no extruder target. And since the current extruder is 1, a change tool command is output. Not immediately obvious what the correct fix is here. A hack might be to set the target extruder to the current extruder when initially parsing a gcode comment.

[dcnewman]

Turns out, this is a S3D bug: it is not specifying -g when running gpx. Specifying -g will resolve the issue.
Using -g indicates it is makerbot-style gcode and makes Tn sticky.

[dcnewman]

After a bit more thought, this really does seem to be a bug. Specifying -g is merely a workaround. A benign comment should not be causing GPX to output a change tool command, particularly one which then ruins the print.

[dcnewman]

Possible fix might be to not do the command if the comment flag is set.

    else if(!gpx->flag.dittoPrinting && gpx->target.extruder != gpx->current.extruder &&
    !(gpx->flag & COMMENT_IS_SET) ) {
        int timeout = gpx->command.flag & P_IS_SET ? (int)gpx->command.p : MAX_TIMEOUT;
        CALL( do_tool_change(gpx, timeout) );
        command_emitted++;
    }

May also be appropriate to do that for the prior if() statement as well?