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

Add weight/cost output to gcode. On the way to #647 #3669

Merged
merged 7 commits into from Mar 10, 2017

Conversation

@lordofhyphens
Copy link
Member

@lordofhyphens lordofhyphens commented Jan 16, 2017

Parking PR here to get feedback, this has been asked for a couple times.

Fixes #647
Fixes #2250

#1766

@lordofhyphens lordofhyphens added this to the 1.3.5 milestone Jan 16, 2017
@lordofhyphens lordofhyphens self-assigned this Jan 16, 2017
@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Jan 16, 2017

The parameters default to 0, which turns off the extra output. Cost is defined as per-kg and as such you need the material density to make it work.

@Patola
Copy link

@Patola Patola commented Jan 16, 2017

Congratulations, that is a thing that everybody I know eagerly awaits for in Slic3r. Little question: from the code it seems that the price is set by extruder, wouldn't it be best if it was set by filament?

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Jan 16, 2017

@Patola
Copy link

@Patola Patola commented Jan 16, 2017

Status bar message? Why not the "Info" area? Or is that the area you are referring to?
image

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Jan 16, 2017

@RafaelEstevamReis
Copy link

@RafaelEstevamReis RafaelEstevamReis commented Jan 17, 2017

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Jan 20, 2017

@RafaelEstevamReis Fair point there; I really should ask a friend of mine who is good at UX to take a once-over of Slic3r's UI.

@RafaelEstevamReis
Copy link

@RafaelEstevamReis RafaelEstevamReis commented Jan 20, 2017

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Jan 21, 2017

There's a box now with the sliced statistics, generated post slice. I think I have the math wrong for cost, but I'm kinda spacing out on the unit conversions.

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Jan 21, 2017

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Jan 24, 2017

Win32 build: https://bintray.com/lordofhyphens/Slic3r/download_file?file_path=slic3r-slice_statistics.2017.01.21.337.716806e.zip

I don't know what's up with Travis. I think I have the weight calculated properly from the volume.

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Jan 25, 2017

I've done some rudimentary testing, this looks feature-complete. I'd appreciate other people giving it a whirl.

@lordofhyphens lordofhyphens requested a review from alranel Jan 25, 2017
@Patola
Copy link

@Patola Patola commented Jan 25, 2017

I tried it, configured one filament accordingly, saved it, imported one object, no deal. Two objects, no deal. It does not seem to be populating the "Sliced Info" frame.
slic3r_521
selection_520

@Patola
Copy link

@Patola Patola commented Jan 25, 2017

Got it to populate the frame after saving the G-Code. I understand the reason why this is this way, but it might be confusing to users.
slic3r_522

@Patola
Copy link

@Patola Patola commented Jan 25, 2017

Just to report some additional tests with material with cost defined and cost not defined: the algorithm worked as intended, in this case I set the bigger object to be "ABS_rosa_175mm" which has no cost or density defined. The Slic3r Info frame considered only the filament with density and cost to perform the calculation.

slic3r_524

@forReason
Copy link

@forReason forReason commented Jan 25, 2017

Really Nice!
How about some advanced Calculations like Energycost (time to print the thing) and maybe for more professional users a general Maintenance/printer startup cost?

@Patola
Copy link

@Patola Patola commented Jan 25, 2017

I just noticed that the calculations seem to be three orders of magnitude off. I used 1.24 g/mm³ for the filament (which is the actual density of PLA) but it said in the Sliced Info that the tiny smiling bishop would weight 4,5 kg.

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Jan 25, 2017

@Patola
Copy link

@Patola Patola commented Jan 25, 2017

my apologies. PLA's density is 1.24 g/cm³, not mm³. That means it is 0.00124 g/mm³. In slicers that have a field for cost I am used to input it in g/cm³, so I entered that into slic3r without even thinking about units. Now the calculation is working

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Jan 25, 2017

@forReason I don't see any value added in putting static costs into the interface, especially since it has very little practical use for most people and is yet another option to clutter the interface (and we are trying to limit those).

(Aka you need to convince me)

@Patola regarding the frame not populating until after gcode export, that is something I might add a tooltip to the box to cover. While I would love to have that information available on preview, I think it would require a lot of refactoring to get it.

So what to do about user error in the case of mixed settings? It should be treating the weight for the part without settings as 0.

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Jan 25, 2017

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Jan 25, 2017

I also don't much care for the (money) placeholder. I think that it's okay for now, but should be replaced with the correct locale symbol when we have that working correctly.

@forReason
Copy link

@forReason forReason commented Jan 26, 2017

@lordofhyphens You are probably right the static value is unnessesary. BUT I really would consider energy costs. by Print-time

@Patola
Copy link

@Patola Patola commented Jan 26, 2017

@lordofhyphens not refactoring is better then. There's things that are much more important right now, I don't think it is a great hassle to wait for the G-Code export to have this data.

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Jan 26, 2017

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Feb 16, 2017

I've taken a cue from @bubnikv and the box now hides itself if you don't have gcode exported. It goes away if you move the model in such away that invalidates things.

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Mar 9, 2017

@alexrj I would like to merge this unless you have an objection.

@platsch
Copy link
Member

@platsch platsch commented Mar 9, 2017

Tested and works so far. Two remarks / suggestions:
is there a reason not to set useful default values? It feels like a rough estimation is better than no estimation...
I'd like to have the length of filament used, didn't check your code, isn't that already calculated to get the volume? if not, it should be easy to do since the filament diameter is known already...?

Downside: even more clutter

@lordofhyphens lordofhyphens force-pushed the lordofhyphens:slice_statistics branch from dfd7e56 to 05e9991 Mar 10, 2017
@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Mar 10, 2017

Added the filament length (which was already exposed) and ensured that the box is hidden on config change or if invalidation happens for background processing.

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Mar 10, 2017

I feel it is better to return something that is obviously wrong so the user can set the appropriate values for their filament (since density of PLA and ABS differ significantly) than provide some "default" which is generally always wrong (and basically silently wrong).

@platsch
Copy link
Member

@platsch platsch commented Mar 10, 2017

Ok, lenght works now, empty defaults is fine. But I noticed 3 other issues:

  • the units are inconsistent. The Density (input) is g/cm³ while the output volume is mm³
  • The UI doesn't update after exporting i.e. the Sliced Info frame is invisible. Only when I click somewhere it appears. No idea what's going on.
  • If the Sliced Info was displayed correctly and I change a setting to trigger re-slicing I end up with this:
    filament_cost_screenshot
@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Mar 10, 2017

…correctly on different platforms.

Changed output units to cm/cm^3

Conflicts:
	lib/Slic3r/GUI/Plater.pm
@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Mar 10, 2017

The issue with the box doing weird things should be gone now, and I've changed the display units to cm and cm^3. If someone else wants it to be mm/mm^3, too bad for them. ;p

@lordofhyphens
Copy link
Member Author

@lordofhyphens lordofhyphens commented Mar 10, 2017

image

Here it is after I changed a configuration option:
image

@lordofhyphens lordofhyphens merged commit 0741ecc into slic3r:master Mar 10, 2017
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@xoan
Copy link
Member

@xoan xoan commented Mar 11, 2017

Nice feature!

Some things about it:

  • Floats could be probably reduced to 2
  • Units could be printed after value (Used filament: X.YZ cm)
@lordofhyphens lordofhyphens deleted the lordofhyphens:slice_statistics branch Nov 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.