fix: post-process re-parse only updates moves, preserves slicer state (fixes #10528)#10529
Open
adele-with-a-b wants to merge 1 commit intobambulab:masterfrom
Open
Conversation
The previous patch replaced the entire GCodeProcessorResult after running
post-processing scripts, clobbering slicer-computed fields (filament_maps,
nozzle_group_result, filament_change_sequence, required_nozzle_HRC,
nozzle_type, extruder_colors, print_statistics, etc.) that cannot be
reconstructed from G-code text alone.
On H2C/H2D, this caused the send-to-printer flow to send a malformed
get_auto_nozzle_mapping request, which the printer rejected with
result="fail", errno=1, surfacing as:
'The printer failed to build the nozzle auto-mapping table
{ code: 1 }. Please refreash nozzle information.'
The bug reproduces with ANY post_process entry — even a no-op like
'cat' — because the trigger is the re-parse path, not the script output.
Fix: only copy 'moves' and 'lines_ends' from the re-parsed result into
the existing m_gcode_result. Everything else is preserved. Preview still
reflects the post-processed toolpath; send-to-printer keeps the
slicer-computed nozzle/filament mapping state intact.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #10528.
PR #9920 (my own, merged as
3722c98a3) introduced a regression: after running post-processing scripts,finalize_gcode()was replacing the entireGCodeProcessorResultwith a fresh parse of the G-code text. That clobbers slicer-computed config-derived state (filament_maps,nozzle_group_result,filament_change_sequence,required_nozzle_HRC,nozzle_type,extruder_colors,print_statistics, etc.) — none of which can be reconstructed from G-code text alone.On H2C/H2D, this broke the send-to-printer flow:
SelectMachineDialog::CheckStatusAndShow()reads the now-emptynozzle_group_resultand callsctrl_get_auto_nozzle_mapping()with malformed filament info, which the printer rejects withresult="fail",errno=1. Users see:Single-extruder printers aren't visibly affected because
CheckStatusAndShowshort-circuits before the mapping query when no right-extruder nozzles are used. State is still being clobbered there too, just not exercised.Change
Only update the fields that reflect G-code text content (
moves,lines_ends). Preserve everything else from the original slicing result:Rvalue-reference binding rather than value is required —
GCodeProcessorResult's copy constructor is implicitly deleted due to amutable std::mutexmember.This preserves #9920's original goal (preview reflects post-processed toolpaths) while fixing the regression.
Tradeoff worth flagging
This assumes
movesandlines_endsare sufficient to make the preview reflect any change a post-process script could make. That's true for scripts that reorder or rewrite extrusion moves within the existing tool-change structure — e.g. BrickLayers. I have not audited the general case.If a post-process script adds or removes tool changes, fields like
filament_change_sequence,print_statistics, andextruder_colorswill now stay tied to the pre-post-process sequence rather than the executed G-code. Consequences I can think of: time estimates, flush-weight calculations (GetFlushWeight()readsfilament_change_sequence), and any per-extruder visualization in the preview would be stale for the post-processed output.My view is this is still the right tradeoff relative to the current bug (which makes the send flow completely non-functional on H2C/H2D with any post_process entry), but it's a narrower correctness property than the original #9920 was aiming for. If there's a known use case where post-process scripts alter tool-change structure and the existing time/flush outputs need to reflect that, happy to rework — would likely mean selectively updating more fields rather than the all-or-nothing approach #9920 originally took.
Testing
post_processentry on an H2C plate (even a no-op likecat) causes the mapping error.post_processentry works fine.Why this was missed in review
My own #9920 was tested on a single-extruder printer where the clobbered state isn't exercised. The bug only surfaces during the send flow on multi-nozzle (H2C/H2D). I should have verified against an H2C before submitting — won't repeat that.