Conversation
There was a problem hiding this comment.
Pull request overview
This PR refines the print dashboard by replacing manual filament usage input with automatic calculation from uploaded G-code files, and enhances project filtering to show only relevant projects based on status and user ownership.
- Implemented automatic filament usage calculation by parsing G-code files
- Changed print submission form to require G-code file upload instead of manual filament input
- Refined project filtering to show only printing, approved, and user's own printed projects
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| src/routes/dashboard/admin/print/utils.server.ts | Added calculateFilamentUsage function to parse G-code and compute filament weight from extrusion values |
| src/routes/dashboard/admin/print/[id]/+page.svelte | Updated form to accept G-code file upload instead of manual filament input field |
| src/routes/dashboard/admin/print/[id]/+page.server.ts | Modified to process uploaded G-code files and calculate filament usage automatically |
| src/routes/dashboard/admin/print/+page.svelte | Added client-side filtering for projects based on status and user ownership, updated status filter options |
| src/routes/dashboard/admin/print/+page.server.ts | Enhanced server-side queries to filter projects by status and ownership, added user filter override for printed status |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (newE > currentE) { | ||
| totalExtruded += newE - currentE; | ||
| } | ||
| currentE = newE; |
There was a problem hiding this comment.
The G-code parser only accounts for positive extrusion differences when newE is greater than currentE. However, it doesn't handle retractions (negative E movements) properly. When newE is less than currentE (which happens during retraction), the currentE is still updated to newE, but the totalExtruded is not adjusted. This can lead to incorrect calculations if the G-code uses absolute extrusion positioning. Consider tracking retractions separately or handling all E value changes to maintain accurate state.
| const eMatch = trimmed.match(/E([0-9.-]+)/); | ||
| if (eMatch) { | ||
| currentE = parseFloat(eMatch[1]); | ||
| } | ||
| } else if (trimmed.startsWith('G1')) { | ||
| const eMatch = trimmed.match(/E([0-9.-]+)/); |
There was a problem hiding this comment.
The regular expression pattern allows multiple dots in numbers (e.g., "1.2.3" would match). This could lead to parseFloat returning NaN for malformed numbers, though the validation at line 227 would catch it. Consider using a more restrictive pattern like /E(-?[0-9]+.?[0-9]*)/ to ensure only valid decimal numbers are matched.
| const eMatch = trimmed.match(/E([0-9.-]+)/); | |
| if (eMatch) { | |
| currentE = parseFloat(eMatch[1]); | |
| } | |
| } else if (trimmed.startsWith('G1')) { | |
| const eMatch = trimmed.match(/E([0-9.-]+)/); | |
| const eMatch = trimmed.match(/E(-?\d+(?:\.\d+)?)/); | |
| if (eMatch) { | |
| currentE = parseFloat(eMatch[1]); | |
| } | |
| } else if (trimmed.startsWith('G1')) { | |
| const eMatch = trimmed.match(/E(-?\d+(?:\.\d+)?)/); |
|
|
||
| return currentlyPrinting; | ||
| } | ||
|
|
There was a problem hiding this comment.
The function lacks documentation explaining its purpose, parameters, return value, and the assumptions it makes about the G-code format. Consider adding a JSDoc comment that explains the calculation method, the assumed filament diameter and density values, and what G-code commands are supported (G1 for extrusion moves, G92 for position resets).
| /** | |
| * Estimates the amount of filament used in a print based on its G-code. | |
| * | |
| * The function parses the provided G-code text, tracking the extruder position (`E` value) | |
| * on supported commands and summing only the positive increases in extrusion length. | |
| * | |
| * Assumptions and calculation details: | |
| * - Filament diameter is assumed to be 1.75 mm. | |
| * - Filament density is assumed to be 1.24 g/cm³ (typical for PLA). | |
| * - G-code units are assumed to be millimeters. | |
| * - Extrusion is assumed to be in absolute mode for the `E` axis. | |
| * - Only the following commands are interpreted: | |
| * - `G1` with an `E` parameter for extrusion moves; increases in `E` are treated as | |
| * additional filament length extruded. | |
| * - `G92` with an `E` parameter to reset the current extruder position. | |
| * - All other commands and parameters are ignored for the purpose of this calculation. | |
| * | |
| * The total extruded filament length (in mm) is converted to volume using the | |
| * cross-sectional area of a 1.75 mm filament, then converted to cm³ and multiplied | |
| * by the assumed density to estimate the mass in grams. | |
| * | |
| * @param gcodeText - Full G-code content as a string, using millimeters and absolute `E`. | |
| * @returns Estimated filament mass in grams. | |
| */ |
| const FILAMENT_DIAMETER = 1.75; | ||
| const FILAMENT_DENSITY = 1.24; |
There was a problem hiding this comment.
The filament diameter (1.75mm) and density (1.24 g/cm³) are hardcoded. These values may vary depending on the filament type (PLA, ABS, PETG, etc.) and manufacturer. Consider making these configurable parameters or documenting which filament type these values represent. The density appears to be for PLA, but this should be explicitly documented.
| parseFloat(filamentUsed.toString()) < 0 | ||
| ) { | ||
| return error(400, { message: 'invalid filament used' }); | ||
| const gcodeText = await gcodeFile.text(); |
There was a problem hiding this comment.
No file size validation is performed on the uploaded G-code file before reading its contents with .text(). Large G-code files could potentially cause memory issues or timeouts. Consider adding a file size check and rejecting files that exceed a reasonable limit (e.g., 10-50 MB) before processing.
| const gcodeText = await gcodeFile.text(); | |
| const MAX_GCODE_SIZE_BYTES = 50 * 1024 * 1024; // 50 MB | |
| const gcodeBlob = gcodeFile as Blob; | |
| if (typeof (gcodeBlob as any).size === 'number' && (gcodeBlob as any).size > MAX_GCODE_SIZE_BYTES) { | |
| return error(413, { message: 'G-code file is too large' }); | |
| } | |
| const gcodeText = await gcodeBlob.text(); |
| allProjects | ||
| .filter((project) => { | ||
| if (project.status === 'printing') return true; | ||
| if (project.status === 'printed' && project.userId === currentUserId) return true; | ||
| return false; | ||
| }) | ||
| .filter((project) => project.name?.toLowerCase().includes(projectSearch.toLowerCase())) |
There was a problem hiding this comment.
The filtering logic duplicates the server-side filtering in +page.server.ts. The allProjects data already has status and userId filtering applied on the server (lines 28-32 in +page.server.ts), but this client-side filter applies the same logic again. This duplication can lead to maintenance issues if the filtering rules need to change. Consider relying on the server-side filtering or documenting why both are necessary.
| allProjects | |
| .filter((project) => { | |
| if (project.status === 'printing') return true; | |
| if (project.status === 'printed' && project.userId === currentUserId) return true; | |
| return false; | |
| }) | |
| .filter((project) => project.name?.toLowerCase().includes(projectSearch.toLowerCase())) | |
| allProjects.filter((project) => | |
| project.name?.toLowerCase().includes(projectSearch.toLowerCase()) | |
| ) |
| return parseInt(userId.toString()); | ||
| }); | ||
|
|
||
| if (statusFilter.length === 1 && statusFilter[0] === 'printed') { |
There was a problem hiding this comment.
The userFilter override on lines 80-82 silently discards any user selections when the status filter is 'printed'. This behavior is not obvious from the UI and could be confusing to users who selected specific users in the filter. Consider either making this restriction clear in the UI or preventing users from selecting user filters when 'printed' is the only status selected.
| if (statusFilter.length === 1 && statusFilter[0] === 'printed') { | |
| if (statusFilter.length === 1 && statusFilter[0] === 'printed' && userFilter.length === 0) { |
| {/each} | ||
| <option value="t1_approved" class="truncate">{projectStatuses['t1_approved'] ?? 'On Print Queue'}</option> | ||
| <option value="printing" class="truncate">{projectStatuses['printing'] ?? 'Being printed'}</option> | ||
| <option value="printed" class="truncate">{projectStatuses['printed'] ?? 'Printed'}</option> |
There was a problem hiding this comment.
Inconsistent indentation: lines 75-76 use tabs while line 77 uses a tab plus spaces. All three option elements should have the same indentation level for consistency.
| <option value="printed" class="truncate">{projectStatuses['printed'] ?? 'Printed'}</option> | |
| <option value="printed" class="truncate">{projectStatuses['printed'] ?? 'Printed'}</option> |
| <input | ||
| name="filament" | ||
| type="number" | ||
| name="gcode" | ||
| type="file" | ||
| class="themed-input-on-box" | ||
| placeholder="50" | ||
| step="0.1" | ||
| min="0" | ||
| accept=".gcode" | ||
| required | ||
| /> |
There was a problem hiding this comment.
The file input lacks client-side validation for file size. Users could attempt to upload very large files, which would only fail after upload and processing. Consider adding a maxlength or size attribute, or using JavaScript to validate file size before submission to provide better user experience.
| for (const line of lines) { | ||
| const trimmed = line.trim().toUpperCase(); | ||
| if (trimmed.startsWith('G92')) { | ||
| const eMatch = trimmed.match(/E([0-9.-]+)/); | ||
| if (eMatch) { | ||
| currentE = parseFloat(eMatch[1]); | ||
| } | ||
| } else if (trimmed.startsWith('G1')) { | ||
| const eMatch = trimmed.match(/E([0-9.-]+)/); | ||
| if (eMatch) { | ||
| const newE = parseFloat(eMatch[1]); | ||
| if (newE > currentE) { | ||
| totalExtruded += newE - currentE; | ||
| } | ||
| currentE = newE; | ||
| } | ||
| } |
There was a problem hiding this comment.
The function processes every line in the G-code file even if most lines don't contain E (extrusion) commands. For large G-code files, this could impact performance. Consider optimizing by checking for the presence of 'E' in the line before running the regex match, or filtering lines that contain 'E' first before processing.
No description provided.