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.

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

M73 R : Set Remaining Time #15549

Merged
merged 3 commits into from
Nov 13, 2019

Conversation

rmoravcik
Copy link
Contributor

This commit will add support for parsing of M73 gcode parameter R (remaining time). The parsed remaining time will be displayed on graphical display instead of calculated ETA.

Parameter R was introduced by PrusaSlicer will address issue #12123.

@LinFor
Copy link
Contributor

LinFor commented Oct 14, 2019

What's the point of using a completion time estimate based on the slicer assumptions in a situation where real statistics are available (real elapsed time)? i just don't understand the use case...

@rmoravcik
Copy link
Contributor Author

@LinFor Well, for example PrusaSlicer is able to calculate very precisely the remaining time if you set you machine limits to slicer (speeds, acceleration, jerk). It is simulating all moves when calculating remaining time.

@LinFor
Copy link
Contributor

LinFor commented Oct 14, 2019

@LinFor Well, for example PrusaSlicer is able to calculate very precisely the remaining time if you set you machine limits to slicer (speeds, acceleration, jerk). It is simulating all moves when calculating remaining time.

Yes, I agree. But this estimation may not take into account some aspects, eg G10/G11, Linear Advance, planner buffer depletion (when speed decreasing to fill buffer) and other.
While we have a real elapsed timer and current progress info (M73 P, of course provided by slicer), what reason to not use real timer and use slicer estimation to calculate ETA?

I think, while we have current progress info (M73 P) and real elapsed timer, we can ignore M73 R as this estimation doesn't add any accuracy to we already have. If M73 P absent - yes M73 R can be used to calculate current print progress, as another alternative - the position in the file - is significantly less accurate.

@comps
Copy link
Contributor

comps commented Oct 14, 2019

While we have a real elapsed timer and current progress info (M73 P, of course provided by slicer), what reason to not use real timer and use slicer estimation to calculate ETA?

Because, depending on the model, estimation based on "real elapsed timer" (a.k.a. gcode line nr.) can also be wildly off if the model differs significantly throughout Z.
For example, if the bottom half of the model takes 100x longer to print than the top half, the estimated remaining time will be very incorrect. A slicer can "look ahead" and account for this.

@LinFor
Copy link
Contributor

LinFor commented Oct 14, 2019

Because, depending on the model, estimation based on "real elapsed timer" (a.k.a. gcode line nr.) can also be wildly off if the model differs significantly throughout Z.

No. That is why the M73 P provided by the slicer is used for calculating ETA, and not the position in the file.
"real elapsed timer" - exactly timer, aka number of seconds past from print starts. Not the "gcode line nr" or other meaning. Current print progress ("gcode line nr") used from M73 P if available or byte position in gcode file (which i consider call "inaccurate").
If we assume that the slicer does not calculate M73 P correctly, then where is the reason to believe that the same slicer correctly predicts M73 R?

@thinkyhead thinkyhead changed the title Added support of M73 parameter R (remaining time) M73 R : Set Remaining Time Oct 15, 2019
@thinkyhead
Copy link
Member

Given that slicers output this command, I suppose we must interpret it. However, it is a good question how to use or combine the internal estimation with the external estimation. Perhaps the firmware should take both values, average them together, and display that!

@randellhodges
Copy link
Contributor

I'm not an embedded developer, so I'm not sure the overhead involved with this suggestion, but, could there be a bit/flag added in set_remaining_time. The function get_remaining_time would use the "old" method as long as that flag isn't set. Then once it is set, it uses the "new" logic?

I think that would support mixed slicers, as well as streaming. Of course, that involves a new bit/flag in memory and an if check on every get call.

@thinkyhead
Copy link
Member

thinkyhead commented Oct 15, 2019

This is interesting in that you could also just set the estimation of total print time once — at the beginning of the print. As the print goes along, you simply subtract the elapsed time from the time estimate. If the estimate was wrong and the time goes to zero before the print is over — or if the difference becomes very great compared to the actual time — you could switch over to the internal estimate.

Or, the time estimate could just periodically get corrected — say just once every 10 minutes — and in the interim you're just subtracting the time elapsed since the latest estimate arrived.

This approach also allows an added correction. If the time estimates are consistently off by 10% then you can include that factor in displaying the estimate.

@comps
Copy link
Contributor

comps commented Oct 27, 2019

What about special cases like M600 in the middle of the print, sitting idle for a few hours? Or an equivalent via M0? Or G4? Will the algorithm recover?

@thinkyhead
Copy link
Member

The print job timer pauses when a job is paused for any reason. Though, not for M4 delays.

@thinkyhead
Copy link
Member

Looks like the remaining_time value needs to be reset to 0 at the start of a print job. The correct place to do kind of thing in Marlin is now the startOrResumeJob function.

@rmoravcik
Copy link
Contributor Author

rmoravcik commented Oct 29, 2019

Looks like the remaining_time value needs to be reset to 0 at the start of a print job. The correct place to do kind of thing in Marlin is now the startOrResumeJob function.

Someting like this?

diff --git a/Marlin/src/Marlin.cpp b/Marlin/src/Marlin.cpp
index 438c70806..ec7c39558 100644
--- a/Marlin/src/Marlin.cpp
+++ b/Marlin/src/Marlin.cpp
@@ -370,6 +370,9 @@ void startOrResumeJob() {
   #if ENABLED(CANCEL_OBJECTS)
     if (!printingIsPaused()) cancelable.reset();
   #endif
+  #if ENABLED(USE_M73_REMAINING_TIME)
+    ui.reset_remaining_time();
+  #endif
   print_job_timer.start();
 }
 
diff --git a/Marlin/src/lcd/ultralcd.h b/Marlin/src/lcd/ultralcd.h
index c54680415..16d49f34b 100644
--- a/Marlin/src/lcd/ultralcd.h
+++ b/Marlin/src/lcd/ultralcd.h
@@ -306,6 +306,7 @@ public:
           static uint32_t remaining_time;
           static void set_remaining_time(const uint32_t r) { remaining_time = r; }
           static uint32_t get_remaining_time() { return remaining_time; }
+          static void reset_remaining_time() { set_remaining_time(0); }
         #endif
       #endif
       static progress_t _get_progress();

@thinkyhead thinkyhead merged commit e110f5a into MarlinFirmware:bugfix-2.0.x Nov 13, 2019
philippniethammer pushed a commit to philippniethammer/Marlin that referenced this pull request Dec 21, 2019
christran206 pushed a commit to christran206/Marlin2.0-SKR-Mini-E3-1.2 that referenced this pull request Dec 30, 2019
@rmoravcik rmoravcik deleted the m73_remaining_time branch March 4, 2020 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants