prusa3d / PrusaSlicer

G-code generator for 3D printers (RepRap, Makerbot, Ultimaker etc.)
https://www.prusa3d.com/prusaslicer/
GNU Affero General Public License v3.0
7.41k stars 1.88k forks source link

Add compatibility for Machine Limits with RepRapFirmware G-code flavor #7347

Closed n8bot closed 2 years ago

n8bot commented 2 years ago

This is pretty self explanatory. I've adapted the emitted G-code to be appropriate for RRF. The time estimates are definitely close enough using the normal Marlin algorithm.

This seems like a well-received change in the RRF community. I've already ben asked to submit a PR.

RRF does not use stealth mode, but it can provide two time estimates and causes no harm afaik.

MachineLimits

xPew commented 2 years ago

It would be indeed a great addition for RRF users! Thanks for the PR :)

bubnikv commented 2 years ago

Thanks, but not for 2.4. We need to release.

P-C-R commented 2 years ago

@bubnikv As far as i know prusa has bought Trilab. They are using RRF controllers aswell;)

ibash commented 2 years ago

RRF does not use stealth mode, but it can provide two time estimates and causes no harm afaik.

FWIW it's possible to create a stealth mode for RRF by enabling/disable stealthchop mid print.

n8bot commented 2 years ago

RRF does not use stealth mode, but it can provide two time estimates and causes no harm afaik.

FWIW it's possible to create a stealth mode for RRF by enabling/disable stealthchop mid print.

Interesting! Yeah, there would be many ways to implement "stealth mode." To get it to automatically switch over, based on the slicer settings, one would have to capture the setting in the start g-code or other custom-gcodes to enable it manually. AFAIK, when you enable "supports stealth mode" all that happens in PrusaSlicer is that it calculates a new set of times, with the second set of values provided. I don't think it will emit the second set of settings to gcode.

ibash commented 2 years ago

Makes sense -- thanks for this change btw, going to be using this locally to help tune print speed. Once my next print completes I can give feedback on accuracy, too (so far it's looking really close).

lukasmatena commented 2 years ago

@n8bot Thanks. Could you please have a look at my branch lm_pr7347? I pulled your commit as it was (I just added a reference to this PR in the commit message) and continued with three more commits that I believe are needed. Can you please check them?

n8bot commented 2 years ago

@lukasmatena It looks good to me! Thanks for fixing it all up. There was one spot you forgot to use the new flavor variable and use the old expanded form but that's just FYI. Maybe you did that for a reason.

Also, RRF does support minimum feedrate but just not the way marlon does. Instead there is a global minimum feedrate defined by "M205 Inn" see here: https://duet3d.dozuki.com/Wiki/M203

Obviously I didn't include that in my implementation. I guess I just thought it wasn't important enough and wanted to change as little as possible. If you'd like to, you could enforce that global minimum feedrate if detected in g-code. Not sure if it's necessary to allow user to set a new global minimum from the settings page but feel free if you wish.

n8bot commented 2 years ago

Sorry, a couple follow-up comments now that I have thought for a few more minutes:

If the global minimum feedrate for RRF is implemented, it probably should be user-inputable in the settings page, because it is unlikely that an M205 command will appear in the gcode to set the minimum global feedrate. That's usually a setting that a user would place in their config.g file that is run on printer startup -- the only chance PrusaSlicer will get to know that value is if the user inputs it, or PrusaSlicer can somehow get the config.g from the nwtworked printer.

Also, in my changed I changed the dscription on the machine limits page to describe how the estimator is newly used for RRF. You may wish to remove/revert the description to normal. Just FYI, in case Prusa doesn't want unneeded reference to RRF.

n8bot commented 2 years ago

@ibash

[...] Once my next print completes I can give feedback on accuracy, too (so far it's looking really close).

How was the accuracy of the time estimate in the end?

ibash commented 2 years ago

I forgot to record it 😅 I think it was in the ballpark though. Running a print now but can run some print simulations to compare in a bit.

ibash commented 2 years ago

For a couple prints...

Slicer: 1h 28m 24s Simulated: 1h 46m 56s

Slicer: 4h 2m 52s Simulated: 4h 42m 20s

n8bot commented 2 years ago

Hmm that's not too bad. I suspect that the too-low error might be caused by the minimum feedrate problem, that was fixed by lukasmatena. So it's entirely possible that they will be even closer to simulation with these latest changes.

P-C-R commented 2 years ago

Slicer time does not include heating up i think

Islam Sharabash @.***> schrieb am Di., 4. Jan. 2022, 20:22:

For a couple prints...

Slicer: 1h 28m 24s Simulated: 1h 46m 56s

Slicer: 4h 2m 52s Simulated: 4h 42m 20s

— Reply to this email directly, view it on GitHub https://github.com/prusa3d/PrusaSlicer/pull/7347#issuecomment-1005104250, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALMPL7AKDJFUV5GFI6EF3RTUUNCIRANCNFSM5IZJFLMQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you commented.Message ID: @.***>

n8bot commented 2 years ago

I don't think the simulation accounts for any heat-up time. But, for sure, the PrusaSlicer simulation does not account for pressure advance so if you use that it could affect things potentially, too.

ibash commented 2 years ago

About to print a benchy, will report back with an actual time. Slicer: 1h 13m 5s Simulation: 1h 22m 22s Actual: 1h 29m 54s

ibash commented 2 years ago

Updated the times for the benchy -- I'm not sure if that takes into account heating time or not. But all in all still fairly close.

n8bot commented 2 years ago

If you'd be willing to test, I'll compile a version with the new changes so you can see if they are more accurate. Otherwise, it could be down to differences in jerk behaviour, or just slight differences or errors in approximation.

It does seem fairly close. Would you mind trying a simulation vs slicer estimate of a very very long print and see how different they are? The simulation seems very close to accurate, so we can I think reasonably judge based off simulation alone.

I'm curious if it will be off by a percentage or a fixed like 30 minutes or so. It seems like it will be out by a percentage, which seems small for short prints but it's actually tremendously off for longer prints. It could be off by a whole day for week-long prints. (Who plans on doing week-long prints!?!?! lolololo me)

ibash commented 2 years ago

Happy to try a new version

Here's another slicer vs simulated datapoint: Slicer: 11h 46m 59s Simulated: 14h 29m 38s

Going to run an even longer one, might be useful to plot these

n8bot commented 2 years ago

Thank you. I will compile a version of stock prusaslicer 2.4.0 with lukasmatena's latest changes and see if that brings the estimate closer.

I hate to ask, but I have to mention it just because we're getting into the testing more: please double-verify that the settings input in the slicer for estimation are identical to those used in the actual print. Make sure no config-override, or other thing is setting them to unexpected values. I don't suspect that this is the case -- I 100% believe that you are reporting things correctly. But we must check just to be sure. Thanks!

ibash commented 2 years ago

@n8bot I've been checking, but good to have a second set of eyes

Screen Shot 2022-01-04 at 2 42 01 PM

In my duet config I have these speeds, and then I override them later to enable stealthchop.

M201 X2500 Y2500 Z100 E1500      ; Accelerations (mm/s^2)
M203 X24000 Y24000 Z900 E3600    ; Maximum speeds (mm/min)
M566 X800 Y800 Z100 E1500        ; Maximum jerk speeds mm/minute

And then overridden with:

M569 P0 D3 V15 H15 ; Enable stealthchop on X
M569 P1 D3 V15 H15 ; Enable stealthchop on Y
M915 X Y T15       ; Coolstep only starts after stealthchop (effectively never)

; Limit speed / acceleration
M201 X1200 Y1200  ; Max Acceleration (mm/s^2)
M203 X7200 Y7200  ; Max Speed (mm/min)
M566 X600 Y600    ; Max "jerk" speed (mm/min)

I ended up plotting the times:

Screen Shot 2022-01-04 at 2 40 51 PM
n8bot commented 2 years ago

I made a windows build of stock 2.4.0 as released, with the addition of the changes from lukaxmatena's branch. I realize now that it seems like you're capable of compiling this on your own, and that you use macos.... lol shit. So are you able to pull the changes from this branch and compile? If not, I can get a build going for macos... just gotta boot up the wife's macbook! lol

Also, thank you for confirming and posting your settings. It all seems good to me. I don't know how stealth chop affects prints, but I doubt it should influence anything like we're seeing.

n8bot commented 2 years ago

Another possible reason for the discrepancy in time could be the jerk policy. I use jerk policy P1, which emulates the behaviour of Marlin. Stock RRF, with policy P0, implements jerk slightly differently: when no other move joins with a move, the feedrate will actually go down to zero, come to a complete stop at the end of a line. This could add up over the course of a print to be the discrepency we see.

Without modifying the PrusaSlicer time estimator code, a way to check would be to send M566 P1 before a simulation and see if it changes anything. I personally use P1 policy because I find the prints better, and no reason to use the other original policy. Maybe the prints are faster, too!

ibash commented 2 years ago

No worries, I can pull and compile.

BTW did a longer simulation: slicer: 47h 1m simulation: 57h 30m 33s

What's interesting is that this was the same model as the 11h/14h datapoint, but with 4 instances instead of 1. The percent it's off is actually the same (18%), and the print time is almost exactly 4x.

This is good since it means there aren't accumulated errors, it's dependent on the set of moves.

n8bot commented 2 years ago

Yes at least things are consistent. We're getting close. It's already usable, because users can get a feel for how off it is and adjust their expectations accordingly.

I'm compiling on macOS right now, so if you don't want to bother yourself you can just wait and I'll post the universal DMG

Otherwise, I suggest you try a test with the new jerk policy P1 like I mentioned in the last post.

Thank you again for doing the testing. I've been very focused on the coding and stuff that I haven't had time to thoroughly run through any numbers like you are. I appreciate it a lot!

n8bot commented 2 years ago

I'm sorry it took me longer than intended to get the build finished. I made a booboo the first time, which prevented macOS from opening the file (because of a mismatch of binary name to app name in the DMG making process arrrg.

Anyway, here's a link to the DMG with lukasmatena's added fixes: https://github.com/n8bot/PrusaSlicer/releases/download/rrftimetest/PrusaSlicer.2.4.0+lm_pr7347macOSuniversal.dmg

Did you get a chance to run a simulation with M566 P1 sent beforehand?

ibash commented 2 years ago

Tested with the new branch and got the same estimated slice times.

Using M566 P1 brought the simulation time down slightly, but only a bit (1h 22m 22s to 1h 19m 32s).

n8bot commented 2 years ago

I think that might be the best we can do (without re-writing the time estimator to specificaly emulate RRF -- not on my to do list for now, lol). There may be other small differences between marlin and RRF, but IMO I don't think that's it.

The PrusaSlicer time estimator uses a fixed (reasonable) value for the lookahead queue (yes, it appears to simulate lookahead slowdowns!).

I know that the lookahead queue that PrusaSlicer uses is bigger than the one I use in RRF2, but it might be smaller than the lookahead queue in RRF3. This might, alone, account for the rest of the discrepancy -- PrusaSlicer might be estimating lookahead slowdowns, when RRF3 does not encounter such slowdowns with the given combination of settings.

There may be other manners that either the estimator, or RRF, differ/stumble/slowdown. Hiccups (yes, the technical term!) in RRF add 50 microseconds each! lol. which is not significant. But other small details like that could all add up.

I will do some tests myself, because it's not fair to unload them all onto you. But one thing I would be curious to experiment with is a variety of settings. Some extremely "slow" and some extremely fast, and see how that influences the discrepancy.

Thank you again for all the help testing. The estimates are sadly not as close as I'd like, but at least they are now relative to the given settings -- so one can get a relative idea of the time differences when changing firmware settings, not just the feedrate in the slicer.

lukasmatena commented 2 years ago

@n8bot

It looks good to me!

Great, thanks for the review.

one spot you forgot to use the new flavor variable

I only spotted one place, which I changed with my latest commit (76278a1). That is a detail though.

there is a global minimum feedrate defined by "M205 Inn" see here: https://duet3d.dozuki.com/Wiki/M203

I know about the M203 Inn gcode, but I did not implement it. Unless I'm mistaken, if there was a non-zero value and PrusaSlicer did not know, it should make the estimate too long, which is opposite of what we see. It should be negligible with respect to what we currently have.

If the global minimum feedrate for RRF is implemented, it probably should be user-inputable in the settings page

In a perfect world it should be, yes. We would still have to defile the UI a bit, which currently accepts separate values for travel and extrusion moves. It's probably not that much work, but to be honest, I don't know if it's worth changing it again at this point.

because it is unlikely that an M205 command will appear in the gcode

Even if M203 I.. does appear, current time estimator will not process it anyway.

So, I don't know. I'm inclined to test and merge it as it is (in lm_pr7347) and fix the time estimator for RRF later, including the addition of min feedrate handling. Any other opinion?

@ibash Thanks.

ibash commented 2 years ago

+1 on bringing this in and improving it for RRF over time later. It'll also make it easier to get other testers on board, there could totally be something funny going on my machine.

n8bot commented 2 years ago

I agree 100%. I didn't think minimum speed was necessary. And you're right, it won't help improve anything.

lukasmatena commented 2 years ago

@n8bot Few more changes in dc36259002b9bd37399c46d1143cbdba078b9b73, if you agree.

n8bot commented 2 years ago

I see no problem. Thanks!

P-C-R commented 2 years ago

One Thing tho. The "issue" with emit to gcode? Could the default be changed ?

Best

n8bot @.***> schrieb am Mi., 5. Jan. 2022, 10:46:

I see no problem. Thanks!

— Reply to this email directly, view it on GitHub https://github.com/prusa3d/PrusaSlicer/pull/7347#issuecomment-1005531446, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALMPL7F2ANPRGX4NN5XX45TUUQHP3ANCNFSM5IZJFLMQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you commented.Message ID: @.***>

bubnikv commented 2 years ago

BTW did a longer simulation: slicer: 47h 1m simulation: 57h 30m 33s

How accurate is the simulation?

The 18% discrepancy is much higher from what we are used to with PrusaSlicer & Marlin.

Also I would expect it to be the other way around, PrusaSlicer possibly estimating a longer time than shorter. It looks to me as if some machine parameter was not handled correctly.

st 5. 1. 2022 v 10:53 odesílatel Peter Roß @.***> napsal:

One Thing tho. The "issue" with emit to gcode? Could the default be changed ?

Best

n8bot @.***> schrieb am Mi., 5. Jan. 2022, 10:46:

I see no problem. Thanks!

— Reply to this email directly, view it on GitHub < https://github.com/prusa3d/PrusaSlicer/pull/7347#issuecomment-1005531446>, or unsubscribe < https://github.com/notifications/unsubscribe-auth/ALMPL7F2ANPRGX4NN5XX45TUUQHP3ANCNFSM5IZJFLMQ

. Triage notifications on the go with GitHub Mobile for iOS < https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675

or Android < https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub .

You are receiving this because you commented.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/prusa3d/PrusaSlicer/pull/7347#issuecomment-1005536221, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMPSI7BGXLZKXH6A4BUE5TUUQIJHANCNFSM5IZJFLMQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

n8bot commented 2 years ago

@P-C-R Lukasmatena changed the emit-to-g-code default: https://github.com/prusa3d/PrusaSlicer/blob/dc36259002b9bd37399c46d1143cbdba078b9b73/src/libslic3r/PrintConfig.cpp#L1617 I totally kept forgetting over and over to change it. Luckily, it's changed by the prusateam so that's the best way it will be handled -- no surprises.

@bubnikv I will do a few tests of my own with various settings to see the range of discrepancy seen with different combinations of settings. I'll also triple-verify that we are accounting for the settings properly in the time-estimator. It seems we are. Or did you mean the user's printer settings not matching the input? I will perform my tests with the "Emit to G-code" option enabled, so we can be sure that the time estimates reflect the same settings.

n8bot commented 2 years ago

It looks like this was merged, so thank you very much!

lukasmatena commented 2 years ago

Yes, I was just going to close the PR. Thanks for the contribution.

n8bot commented 2 years ago

Lol. I barely did anything. Thank you for being open to contributions.

ibash commented 2 years ago

Just checked an actual print, prusaslicer estimated 55m, actual print was 1h 8m -- pretty accurate. I wonder if duet simulations are off.

n8bot commented 2 years ago

I just completed a print:

PS estimate: 3h 57m, actual time: 4h 5m.

However, it had a peculiarly slow perimeter speed: slower than the jerk speed. So, the calculations are much easier -- there is no accel or decel phase in any of the perimeter moves.

I'm reprinting the exact same print, but with faster perimeter speeds: greater than the jerk speed. Now this will have all the normal accel decel phases. The estimate for the new print is 1h 43m. I suspect it will be a larger discrepancy than 4%. I'll update in an hour or so when the print completes.

n8bot commented 2 years ago

The second print completed, with better-than-expected results:

PS estimate: 1h 43m, actual time: 1h 46m.

Which is again, less than 4% under.

n8bot commented 2 years ago

4 instances at once of the print above:

PS estimate: 6h 38m, actual time: 6h 49m

All of my estimates so far have been less than 4% under, often less than 3% off.