Bugfix: use Lift-z option for 2. extruder #3385

[platsch]

Bugfix for #3385

[lordofhyphens]

Looks good to me, earlier concern seems to be a non-issue w/r/t accidental head crashes.

Could you augment /~https://github.com/alexrj/Slic3r/blob/master/t/retraction.t#L204 and add a regression test to make sure that it uses the selected extruder? Probably just a copy/paste with two different extruders defined instead of just one define the current extruder to be 2 and check to make sure it matches :)

[platsch]

Yes, it's not forgotten, just give me a few days to find the time :-)

[lordofhyphens]

Thanks :) I know how precious time is.

"Nothing unreal exists." - Kiri-kin-tha's First Law of Metaphysics.

On Wed, Jul 6, 2016 at 11:55 AM, platsch notifications@github.com wrote:

Yes, it's not forgotten, just give me a few days to find the time :-)

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#3392 (comment), or mute
the thread
/~https://github.com/notifications/unsubscribe/AAB8CpyTpV7ZP9W2nSNvxINfX5retdb3ks5qS94dgaJpZM4JC_YG
.

[lordofhyphens]

Also, could you rebase this back onto master? It'd be nice to have the travis build passing :)

[platsch]

Ok, I added the testcase you suggested as an additional case. Since this doesn't check for the actual values, I also modified the 1. test /~https://github.com/alexrj/Slic3r/blob/master/t/retraction.t#L52 to keep track of the lift value between extruder switches.

Rebased to master, compiling and testing runs fine, but I'm not able to execute Slic3r, fails with Caught C++ exception of unknown type at /Slic3r/lib/Slic3r/Config.pm line 92. looks like 30139fd breaks something...

[lordofhyphens]

@alexrj had a commit that broke handling of config entries that aren't enumerated in PrintConfig.hpp. The load/save was ported to C++ and exceptions thrown weren't caught. You can rebase to the branch in #3432 to figure out what is in your config file that Slic3r is choking on (then rebase back to master when when it is sorted out).

[platsch]

Ah, it was an option in my config, leftover from testing a different branch. Looks good, think you can merge the PR.

[lordofhyphens]

Any comments/concerns, @alexrj, about this PR?

[lordofhyphens]

Had forgotten to check the changes in my editor to make sure that spaces were used instead of tabs, everything looks good+clean.

@alexrj Unless there's a specific reason to use the old behavior (all retraction settings using the first extruder's settings) that hasn't been considered, I recommend that this get pulled at the earliest opportunity as a squash merge.

[lordofhyphens]

Any reason why this changed in the PR branch?

[platsch]

Yes. With the old parameter set [15, 16]the retract_lift_below test doesn't fail for the second extruder even if only the first extruder config is used incorrectly. The test value for the second extruder must be lower for the below test and higher for the above test to test for the correct behaviour of the second extruder.