aligator / GoSlice

This is an experimental slicer for 3d printing written in Go also usable as modular slicing lib.
Apache License 2.0
108 stars 16 forks source link

adding --fan-speed option. Allows multiple fan speeds based on layer … #20

Closed kpfaulkner closed 4 years ago

kpfaulkner commented 4 years ago

Related to issue #18 .

Adds CLI argument --fan-speed which accepts a string in key=value,key=value etc etc format. Example: --fan-speed 3=20,10=50,60=255 This means at layer 3, primary fan has speed setting 20. At layer 10, change primary fan to 50. At 60, change to 255 etc.

While poking around the source base, I didn't see any explicit parameter validation steps, so the Set() method for this new parameter is a little lengthy but does validation there. There are bounds checks (layer >=0) and speed has to be 0<=x<=255. If you'd prefer a different way to do the validation, just say the word.

If no speed parameters are set, then it defaults back to at layer 2 set the speed to 255 (as per existing functionality).

One thing that I'm wondering about is at some stage, regardless what the user specifies, should we set the primary fan to full speed? ie, once we hit layer X (whatever that is) then we set the fan to full since we've determined this is a big enough print that full speed is warranted? Thoughts?

aligator commented 4 years ago

Hi,

Thank you for the PR.

First of all, merge my master into your branch, you are working on a bit outdated code (the merge conflicts are minimal) :-)

But yes, currently there is no validation at all. I think it would be great to add the validation in a way which is easy to use by all option values. I haven't looked into it, yet.

One thing that I'm wondering about is at some stage, regardless what the user specifies, should we set the primary fan to full speed? ie, once we hit layer X (whatever that is) then we set the fan to full since we've determined this is a big enough print that full speed is warranted? Thoughts?

I don't think we should do this, as full fan speed is not always the best. So the user should just configure how he needs it.

(Also later the fan speed will be coupled to the type of the currently printed paths: eg. if it is normal infill, less speed, if it is bridging full speed,...)

Also as the print speed depends heavily on the used Filament type, please put the fan-options to the filament options. As reference for options, I would suggest to take a look at the PrusaSlicer, as I like how the options are structured there.

I will review your code in detail later, when I have more time.

Thanks you :-)

aligator commented 4 years ago

For your information: just fixed an error in the test which was caused by the fact that maps in go do not guarantee the order. That's why the test sometimes failed.