CURA-13249 Merge header with first GCode part - #2350
CURA-13249 Merge header with first GCode part#2350Erwan MATHIEU (wawanbreton) wants to merge 6 commits into
Conversation
Test Results31 tests 31 ✅ 5s ⏱️ Results for commit 27ef614. ♻️ This comment has been updated with latest results. |
Erwan MATHIEU (wawanbreton)
changed the title
Casper Lamboo (casperlamboo)
left a comment
Casper Lamboo (casperlamboo)
left a comment
There was a problem hiding this comment.
Don't fully understand why this is needed, but code looks good
The front-end saves the GCode parts in a list, and the post-processing plugins expect this list to be in a certain order, with the first elements containing specific parts, namely the header, init gcode and start gcode. With the new structure we had an intermediate part inserted, so scripts were confused. Other option would be to change the post-processing scripts, but that would require more effort, and possible be annoying because the start GCode is optional. |
Remco Burema (rburema)
left a comment
Remco Burema (rburema)
left a comment
There was a problem hiding this comment.
I'm not sure how much I like this; but data-conversions/communication like this is always a bit messy.
| header_part->stream() << header; | ||
|
|
||
| communication->sendGCodePart(getFileHeader(is_extruder_used_bool, filaments_volumes, materials_ids)); | ||
| std::shared_ptr<GCodePart> first_gcode_part = gcode_parts_.front(); |
There was a problem hiding this comment.
GCode-parts can't be empty here right?
There was a problem hiding this comment.
Well, I don't think in practice it could happen, because even if the print is somehow empty, you always have some init gcode.
Since I have to change this PR anyway, the new solution should be a bit cleaner |
Erwan MATHIEU (wawanbreton)
left a comment
Erwan MATHIEU (wawanbreton)
left a comment
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'C++ Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: 27ef614 | Previous: b7a88b6 | Ratio |
|---|---|---|---|
InfillTest/Infill_generate_connect/1/400 |
0.33563716773107233 ms/iter |
0.18062391062060312 ms/iter |
1.86 |
InfillTest/Infill_generate_connect/0/400 |
5.838558437037339 ms/iter |
2.909563363805862 ms/iter |
2.01 |
InfillTest/Infill_generate_connect/1/800 |
45.46038996666842 ms/iter |
23.766269894738063 ms/iter |
1.91 |
InfillTest/Infill_generate_connect/0/800 |
152.33539737499768 ms/iter |
78.34916389999762 ms/iter |
1.94 |
InfillTest/Infill_generate_connect/1/1200 |
423.55207833331104 ms/iter |
218.8825421999809 ms/iter |
1.94 |
WallTestFixture/generateWalls/3 |
6.4509292896532155 ms/iter |
3.568155140683925 ms/iter |
1.81 |
WallTestFixture/generateWalls/15 |
21.084466256420107 ms/iter |
12.671600184613943 ms/iter |
1.66 |
WallTestFixture/generateWalls/9999 |
54.91305792855071 ms/iter |
33.65414356521203 ms/iter |
1.63 |
WallTestFixture/InsetOrderOptimizer_getRegionOrder/3 |
0.010824664347937813 ms/iter |
0.005744756664756113 ms/iter |
1.88 |
WallTestFixture/InsetOrderOptimizer_getRegionOrder/15 |
0.05756158123513372 ms/iter |
0.031507682122777624 ms/iter |
1.83 |
WallTestFixture/InsetOrderOptimizer_getRegionOrder/9999 |
0.16899279936228323 ms/iter |
0.09015681146637872 ms/iter |
1.87 |
WallTestFixture/InsetOrderOptimizer_getInsetOrder/3 |
0.0010335052696796136 ms/iter |
0.0005763190908596189 ms/iter |
1.79 |
WallTestFixture/InsetOrderOptimizer_getInsetOrder/15 |
0.005388488792322701 ms/iter |
0.0030026841213247 ms/iter |
1.79 |
WallTestFixture/InsetOrderOptimizer_getInsetOrder/9999 |
0.011500791124788766 ms/iter |
0.006975430309322269 ms/iter |
1.65 |
HolesWallTestFixture/InsetOrderOptimizer_getRegionOrder/3 |
0.004831234118329971 ms/iter |
0.002428634857088684 ms/iter |
1.99 |
HolesWallTestFixture/InsetOrderOptimizer_getRegionOrder/15 |
0.023198506539392424 ms/iter |
0.01243990782972648 ms/iter |
1.86 |
HolesWallTestFixture/InsetOrderOptimizer_getRegionOrder/9999 |
0.276288205836527 ms/iter |
0.14599852168420943 ms/iter |
1.89 |
HolesWallTestFixture/InsetOrderOptimizer_getInsetOrder/3 |
0.00025585087677546767 ms/iter |
0.0001323251706925846 ms/iter |
1.93 |
HolesWallTestFixture/InsetOrderOptimizer_getInsetOrder/15 |
0.0011189081001060416 ms/iter |
0.0005702139584364888 ms/iter |
1.96 |
HolesWallTestFixture/InsetOrderOptimizer_getInsetOrder/9999 |
0.011381047903528266 ms/iter |
0.006150435554252666 ms/iter |
1.85 |
SimplifyTestFixture/simplify_local |
1.202019796358207 ns/iter |
0.665793831999963 ns/iter |
1.81 |
SimplifyTestFixture/simplify_slot_noplugin |
4.2175246013942305 ns/iter |
1.5874922564560308 ns/iter |
2.66 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: Jelle Spijker (@jellespijker) Erwan MATHIEU (@wawanbreton) Casper Lamboo (@casperlamboo) HellAholic
3 participants
Instead of sending the GCode header as a separate part, we now send it with the very first part of the actual GCode commands, so that it matches the previous behavior and should allow the post-processing scripts to work without having to fix them all.
Since I am not very familiar with the post-processing scripts, I assume this is the correct fix but the result looks a bit weird to me, so please double-check.
CURA-13249