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.

Sign up for GitHub

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

Jump to bottom

[Brainstorming] Warn if any of the objects exceed the printing area #1254

Closed
wants to merge 13 commits into from
Closed

[Brainstorming] Warn if any of the objects exceed the printing area #1254

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
caf4e84
Added docs-tfg and implemented functionality of warning in case objec…
Feb 11, 2016
9e07a3b
Added functionality to warn in case object exceeds printing area
Feb 17, 2016
d89319c
Completed requirementes to add Slic3r as a plugin
Feb 17, 2016
201b572
Solved problem with character '}' at static/js/viewmodels/file.js
Feb 18, 2016
a92b7e3
Changed requirements and solution file for Slic3r integration
Feb 22, 2016
0edbcf3
Completed requirements and solutions for Slic3r integration and added…
Feb 27, 2016
29a821c
Removed unrelated folders and files for pull request
Feb 29, 2016
e742e74
Update analysis.py
Javierma Mar 5, 2016
3baf9d7
Analysis now done in frontend, taking also into account centre origin
Mar 8, 2016
95b6eba
Changed message shown to user using sprintf and improved 'if' checks
Apr 4, 2016
b0cfa28
Identation issues solved and code improved
Apr 11, 2016
413655f
Changed to detect in case of two extruders
May 12, 2016
cd6688b
Removed previousE
May 12, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/octoprint/filemanager/analysis.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

import octoprint.util.gcodeInterpreter as gcodeInterpreter

#Added to check if object exceeds printing area
from octoprint.printer.profile import PrinterProfileManager
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't actually be needed if you change to the suggested approach of instead tracking the min/max coordinates of the object and then utilize that information in the frontend to display the warning instead. Also, including this here without accounting for it in the unit tests breaks those, which is why your PR is currently marked by Travis as broken.


class QueueEntry(collections.namedtuple("QueueEntry", "path, type, location, absolute_path, printer_profile")):
"""
Expand Down Expand Up @@ -258,8 +260,9 @@ def throttle():

self._gcode = gcodeInterpreter.gcode()
self._gcode.load(self._current.absolute_path, self._current.printer_profile, throttle=throttle_callback)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove that white space :)

result = dict()
result["warning"]=self._gcode.getWarning()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where the min/max coordinates of the object should instead be transferred :)

if self._gcode.totalMoveTimeMinute:
result["estimatedPrintTime"] = self._gcode.totalMoveTimeMinute * 60
if self._gcode.extrusionAmount:
Expand Down
21 changes: 20 additions & 1 deletion src/octoprint/static/js/app/viewmodels/files.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,17 @@ $(function() {
}
}
output += gettext("Estimated Print Time") + ": " + formatDuration(data["gcodeAnalysis"]["estimatedPrintTime"]) + "<br>";
if(self.loginState.isUser()&&data["gcodeAnalysis"]["warning"])
{
var warning = "<p>" + gettext("Object exceeds printing area") + "</p>";
warning += pnotifyAdditionalInfo("<pre>Object or objects in file '"+data["name"]+"' exceed/s the printing area</pre>");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need the additional information here? Does it really add anything? The file name could be included in the initial message (use sprintf so that the translation is still generic and only contains a placeholder for the filename, there are examples throughout the code on how to do that), and the rest of the message is just repetition really. It might make sense to include information which axes exceed the build volume (x, y or z, or maybe more than one?).

new PNotify({
title: "Object dimensions",
text: warning,
type: "warning",
hide: false
});
}
}
if (data["prints"] && data["prints"]["last"]) {
output += gettext("Last Printed") + ": " + formatTimeAgo(data["prints"]["last"]["date"]) + "<br>";
Expand Down Expand Up @@ -501,8 +512,16 @@ $(function() {
self.uploadSdButton = $("#gcode_upload_sd");
if (!self.uploadSdButton.length) {
self.uploadSdButton = undefined;
if (_.endsWith(filename.toLowerCase(), ".stl")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this block have to do with warning about a model exceeding the print area? Merge error? Please remove :)

self.slicing.show(location, filename);
}

if (data.result.done) {
$("#gcode_upload_progress .bar").css("width", "0%");
$("#gcode_upload_progress").removeClass("progress-striped").removeClass("active");
$("#gcode_upload_progress .bar").text("");
}
}

self.uploadProgress = $("#gcode_upload_progress");
self.uploadProgressBar = $(".bar", self.uploadProgress);

Expand Down
11 changes: 11 additions & 0 deletions src/octoprint/util/gcodeInterpreter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ def __init__(self):
self.progressCallback = None
self._abort = False
self._filamentDiameter = 0
#Parameters for object size checking
self.warn=False

def load(self, filename, printer_profile, throttle=None):
if os.path.isfile(filename):
Expand Down Expand Up @@ -120,6 +122,13 @@ def _load(self, gcodeFile, printer_profile, throttle=None):
z = getCodeFloat(line, 'Z')
e = getCodeFloat(line, 'E')
f = getCodeFloat(line, 'F')
#Check if x,y or z exceeds printing area
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... that is a bit backwards actually. This way you'll only get a warning if the object exceeds the size of the print area of the default printer profile basically, but not for the currently selected one. So if you get the initial analysis done for a 20x20x20 build volume but then select a printer profile with 10x10x10, it will not be able to detect that it might not fit after all. The gcode interpreter is only run once for each file since it is a very cpu expensive operation. So it should depend on the printer profile as little as possible. In fact, it's only used since we have to assume some starting feedrate if no F parameter is presented, which should rarely happen at all.

I'd rather suggest making the bounding box of the object (min/max X, Y, Z coordinate) part of the analysis result, and then utilizing that in the frontend only to display the notification. This would also have the additional advantage to allow for utilizing that information differently (e.g. for some sort of low res preview).

That would mean adjusting the gcode interpreter to track minimum and maximum coordinates on all three axes, persist that in the file's metadata and then use that data in the frontend only to display a message on load by calculating width, height, length on the fly and comparing with currently selected (!) printer profile.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @nophead just pointed out, you'll also need to take care of start/end gcode situations, e.g., the print job contains start gcode that moves the nozzle outside of the print volume (usually X/Y exceeding the boundaries) and extrudes a bit to prime it. This will be tricky to tackle.

A basic approach would be to only keep track of the min/max coordinates of movements that contain extrusion (delta-E != 0) and are actually movements (so delta-X/Y/Z != 0). Might still cause false positives though if for example the start gcode moves the nozzle from the outside position to the start of the model while priming further. Problematic, since we cannot distinguish printing from preparing to print just from the GCODE.

width=float(printer_profile["volume"]["width"])
depth=float(printer_profile["volume"]["depth"])
height=float(printer_profile["volume"]["height"])
if not self.warn and (x>width or y>depth or z>height):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only works for print beds which have their (0,0) origin in the lower left corner of the print bed. This is not necessarily the case, it could also be the center of the print bed, in which case a simple check if x/y/z are larger than width/depth/height would produce a false negative (imagine a printbed of 20x20 cm, center in the middle, so 10cm up and left from the lower left corner - an object with max X coordinate of 19 would definitely not fit since that would be 19cm to the right of the origin, so actually 9cm off the bed, but the code would not detect that).

self.warn=True
self._logger.warn("Object is bigger than the printing area")
oldPos = pos
pos = pos[:]
if posAbs:
Expand Down Expand Up @@ -271,6 +280,8 @@ def _load(self, gcodeFile, printer_profile, throttle=None):
def _parseCuraProfileString(self, comment, prefix):
return {key: value for (key, value) in map(lambda x: x.split("=", 1), zlib.decompress(base64.b64decode(comment[len(prefix):])).split("\b"))}

def getWarning(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not Java, we don't need getters for everything :) (Yes, I know, I've done that myself, I'm trying to get rid of such things while refactoring)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A further complication is that start end gcode segments might move outside
the maximum print volume.

On 4 March 2016 at 13:19, Gina Häußge notifications@github.com wrote:

In src/octoprint/util/gcodeInterpreter.py
#1254 (comment):

@@ -271,6 +280,8 @@ def _load(self, gcodeFile, printer_profile, throttle=None):
def _parseCuraProfileString(self, comment, prefix):
return {key: value for (key, value) in map(lambda x: x.split("=", 1), zlib.decompress(base64.b64decode(comment[len(prefix):])).split("\b"))}

  • def getWarning(self):

This is not Java, we don't need getters for everything :) (Yes, I know,
I've done that myself, I'm trying to get rid of such things while
refactoring)

—
Reply to this email directly or view it on GitHub
https://github.com/foosel/OctoPrint/pull/1254/files#r55028146.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that probably was rather meant for the "this is a bit backwards" comment further up? But yes, you are right.. I'll add a comment up there.

return self.warn

def getCodeInt(line, code):
n = line.find(code) + 1
Expand Down