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

Stop print timer with M105/M109 #3113

Merged

Conversation

jbrazio
Copy link
Contributor

@jbrazio jbrazio commented Mar 8, 2016

This is a proposed fix for #3061.
Three hooks have been set in order to stop the print timer:

  1. M105
  2. M109
  3. disable_all_heaters()

Note: M105/M109 hooks only stop the print timer when the temperature of the last extruder is set to zero.

@jbrazio jbrazio changed the title Stop print timer at M105/M109 Stop print timer with M105/M109 Mar 8, 2016
@thinkyhead
Copy link
Member

There's a very remote possibility that in a multi-extruder system, one extruder may finish its part and shut off, while the other continues. If the order of operations is COOL 1, HEAT 2 instead of HEAT 2, COOL 1, then the timer would be stopped by this code. I don't really think this is a problem. In any real, working print system, there should never be a time during a print when all heaters are off.

Should other commands, like M81 also turn off the print timer?

@@ -3944,16 +3957,15 @@ inline void gcode_M105() {
* Rxxx Wait for extruder(s) to reach temperature. Waits when heating and cooling.
*/
inline void gcode_M109() {
float temp;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be float temp = 0; because it's comparing with 0 below…

@Blue-Marlin
Copy link
Contributor

This clock is also used for sd-printing. Does it need changes there?
M109 is not a very reliable criterion for starting the job-timer. I never realized it's also working for USB-prints because I always wait only for M190. For the nozzles I use M104.
How can the clock be reset? Currently it seems to add the time of a second print?

This PR seems to improve the current behaviour. But the concept of starting/stopping it is questionable because most non-3d-printer uses of Marlin do not need any temperature. (Start-g-code? Buffer usage? State?)

@jbrazio
Copy link
Contributor Author

jbrazio commented Mar 9, 2016

This code is indeed solving half of the problem. In fact the print timer code is messy, for instance it is only available if SDCARD is defined, and this flag isn't checked in half of the places where Marlin deals with the timer.

There's is also another issue I found when working in this PR but I'm preparing a post about it and will ask for your opinion there.

Nevertheless there are here some loose ends and a couple more hooks have to be introduced into this PR.

@jbrazio jbrazio mentioned this pull request Mar 9, 2016
print_job_stop_ms = millis();
LCD_MESSAGEPGM(WELCOME_MSG);
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this code repeating we might as well make a function and call it in both places:

bool finish_print_job() {
  if (print_job_start_ms) {
    for (int i = 0; i < EXTRUDERS; i++) if (degTargetHotend(i) > 0) return false;
    print_job_stop_ms = millis();
    return true;
  }
  return false;
}

In gcode_M104:

  finish_print_job();

In gcode_M109:

  if (finish_print_job()) LCD_MESSAGEPGM(WELCOME_MSG);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I'll take the opportunity to rework the overall code a bit a implement a start_print_job() function also.

@thinkyhead
Copy link
Member

I agree with @Blue-Marlin that it's tricky to determine when a print starts or stops. But since we are already making these timers dependent on the wait-for-temperature routines, I don't think it's too far outside the scope to also turn off the timer in the case of all temperatures being turned off. We can also re-set the timer to zero in the case when all heaters are currently turned off, but one or more of them is turned on with a wait option.

Generally, host software will keep its own print timer anyway. This timer is especially for SD printing, but since it hooks into the wait-for-heating stuff it also comes in handy when you have a host that doesn't keep a job timer (if there are any).

@jbrazio
Copy link
Contributor Author

jbrazio commented Mar 11, 2016

This is indeed purely a cosmetic change for users who have SDCARD enabled and print from a host as the timer will keep running after everything is finished.

@jbrazio
Copy link
Contributor Author

jbrazio commented Mar 12, 2016

Fixed bug: If during the print the nozzle temp is changed by a M109 the timer would reset itself.
Improvement: I've done a rework of print timer in Marlin, now all actions related with the timer have to go through three functions: print_job_start(), print_job_timer() or print_job_stop(); implemented all the feedback received in this thread.

@AnHardt
Copy link
Contributor

AnHardt commented Mar 20, 2016

Currently M109 does not wait for cooling with 'R'.
Clock functions are ok with sd and temperature triggering.

@AnHardt
Copy link
Contributor

AnHardt commented Mar 20, 2016

Sorry. Works. My target-temp was below EXTRUDE_MINTEMP/2. So that is also tested as working. ;-)

@jbrazio
Copy link
Contributor Author

jbrazio commented Mar 20, 2016

Thanks !
@AnHardt Do you confirm this PR is working properly ?

@AnHardt
Copy link
Contributor

AnHardt commented Mar 20, 2016

Yes.

thinkyhead added a commit that referenced this pull request Mar 21, 2016
@thinkyhead thinkyhead merged commit 06332f2 into MarlinFirmware:RCBugFix Mar 21, 2016
@jbrazio jbrazio deleted the bugfix/3061-stop-print-time-counter branch March 21, 2016 21:20
@boelle boelle mentioned this pull request May 25, 2016
@jbrazio jbrazio modified the milestone: 1.1.0 Jul 18, 2016
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.

4 participants